> -----Original Message-----
> From: Parthasarathy Bhuvaragan
> Sent: Tuesday, 26 January, 2016 09:01
> To: da...@davemloft.net
> Cc: netdev@vger.kernel.org; tipc-discuss...@lists.sourceforge.net; Jon
> Maloy; ma...@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.bhuvara...@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
