netdev - Re: [PATCH v4 3/3] RDS: make sure not to loop forever inside rds_send

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20150407.172607.869971800253661690.davem@davemloft.net>
Date:	Tue, 07 Apr 2015 17:26:07 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	sowmini.varadhan@...cle.com
Cc:	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	chien.yen@...cle.com, rds-devel@....oracle.com, agrover@...hat.com,
	clm@...com, zab@...bo.net, ajaykumar.hotchandani@...cle.com,
	sergei.shtylyov@...entembedded.com
Subject: Re: [PATCH v4 3/3] RDS: make sure not to loop forever inside
 rds_send_xmit

From: Sowmini Varadhan <sowmini.varadhan@...cle.com>
Date: Tue,  7 Apr 2015 16:38:04 -0400

> @@ -157,6 +160,17 @@ int rds_send_xmit(struct rds_connection *conn)
>  	}
>  
>  	/*
> +	 * we record the send generation after doing the xmit acquire.
> +	 * if someone else manages to jump in and do some work, we'll use
> +	 * this to avoid a goto restart farther down.
> +	 *
> +	 * we don't need a lock because the counter is only incremented
> +	 * while we have the in_xmit bit held.
> +	 */
> +	conn->c_send_gen++;
> +	send_gen = conn->c_send_gen;

This increment does need to either be changed to be an atomic_t
or covered by a lock.

Otherwise two concurrent callers can both try to increment it at
the same time, and it only effectively increments once.  That's
corrupted state and will break all of the new logic added here.

Still very unhappy with this patch series submission, as I still find
new problems every time I look at these changes.  Are you evaluating
them and double checking all of the claims in the commit log message
and comments, and logic in these, or are you just passing them off
upstream after testing and leaving the checking to people like me?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html