netdev - RE: [PATCH net] tipc: fix connection abort during subscription cancel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <A2BAEFC30C8FD34388F02C9B3121859D223C0A88@eusaamb103.ericsson.se>
Date:	Tue, 26 Jan 2016 16:10:10 +0000
From:	Jon Maloy <jon.maloy@...csson.com>
To:	Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@...csson.com>,
	"davem@...emloft.net" <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"tipc-discussion@...ts.sourceforge.net" 
	<tipc-discussion@...ts.sourceforge.net>,
	"maloy@...jonn.com" <maloy@...jonn.com>,
	Ying Xue <ying.xue@...driver.com>,
	Richard Alpe <richard.alpe@...csson.com>,
	Anders Widell <anders.widell@...csson.com>
Subject: RE: [PATCH net] tipc: fix connection abort during subscription
 cancel



> -----Original Message-----
> From: Parthasarathy Bhuvaragan
> Sent: Tuesday, 26 January, 2016 09:01
> To: davem@...emloft.net
> Cc: netdev@...r.kernel.org; tipc-discussion@...ts.sourceforge.net; Jon
> Maloy; maloy@...jonn.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@...csson.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