Message ID | 1453816862-30289-1-git-send-email-parthasarathy.bhuvaragan@ericsson.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
> -----Original Message----- > From: Parthasarathy Bhuvaragan > Sent: Tuesday, 26 January, 2016 09:01 > To: davem@davemloft.net > Cc: netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; Jon > Maloy; maloy@donjonn.com; Ying Xue; Richard Alpe; Anders Widell > Subject: [PATCH net] tipc: fix connection abort during subscription cancel > > In 'commit 7fe8097cef5f ("tipc: fix nullpointer bug when subscribing to > events")', we terminate the connection if the subscription creation fails. > In the same commit, the subscription creation result was based on the value > of the >subscription pointer (set in the function) instead of the return code. > > Unfortunately, the same function tipc_subscrp_create() handles subscription > cancel request. This sounds very strange, but seems to be true. I would add a comment that this misnomer will be addressed in a later commit series. >For a subscription cancellation request, the subscription > pointer cannot be set. Thus if a subscriber has several subscriptions and > cancels any of them, the connection is terminated. > > In this commit, we terminate the connection based on the return value of > tipc_subscrp_create(). > Fixes: commit 7fe8097cef5f ("tipc: fix nullpointer bug when subscribing to > events") > > Signed-off-by: Parthasarathy Bhuvaragan > <parthasarathy.bhuvaragan@ericsson.com> > --- > net/tipc/subscr.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c index > 350cca33ee0a..72bb7cdc85aa 100644 > --- a/net/tipc/subscr.c > +++ b/net/tipc/subscr.c > @@ -293,11 +293,11 @@ static void tipc_subscrb_rcv_cb(struct net *net, int > conid, > struct tipc_subscription *sub = NULL; > struct tipc_net *tn = net_generic(net, tipc_net_id); > > - tipc_subscrp_create(net, (struct tipc_subscr *)buf, subscriber, > &sub); > - if (sub) > - tipc_nametbl_subscribe(sub); > - else > - tipc_conn_terminate(tn->topsrv, subscriber->conid); > + if (tipc_subscrp_create(net, (struct tipc_subscr *)buf, subscriber, > + &sub)) Maybe rename subscription to "subscrb", to make the if clause fit on one line? Reviewed-by: Jon ///jon > + return tipc_conn_terminate(tn->topsrv, subscriber->conid); > + > + tipc_nametbl_subscribe(sub); > } > > /* Handle one request to establish a new subscriber */ > -- > 2.1.4
diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c index 350cca33ee0a..72bb7cdc85aa 100644 --- a/net/tipc/subscr.c +++ b/net/tipc/subscr.c @@ -293,11 +293,11 @@ static void tipc_subscrb_rcv_cb(struct net *net, int conid, struct tipc_subscription *sub = NULL; struct tipc_net *tn = net_generic(net, tipc_net_id); - tipc_subscrp_create(net, (struct tipc_subscr *)buf, subscriber, &sub); - if (sub) - tipc_nametbl_subscribe(sub); - else - tipc_conn_terminate(tn->topsrv, subscriber->conid); + if (tipc_subscrp_create(net, (struct tipc_subscr *)buf, subscriber, + &sub)) + return tipc_conn_terminate(tn->topsrv, subscriber->conid); + + tipc_nametbl_subscribe(sub); } /* Handle one request to establish a new subscriber */
In 'commit 7fe8097cef5f ("tipc: fix nullpointer bug when subscribing to events")', we terminate the connection if the subscription creation fails. In the same commit, the subscription creation result was based on the value of subscription pointer (set in the function) instead of the return code. Unfortunately, the same function tipc_subscrp_create() handles subscription cancel request. For a subscription cancellation request, the subscription pointer cannot be set. Thus if a subscriber has several subscriptions and cancels any of them, the connection is terminated. In this commit, we terminate the connection based on the return value of tipc_subscrp_create(). Fixes: commit 7fe8097cef5f ("tipc: fix nullpointer bug when subscribing to events") Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com> --- net/tipc/subscr.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)