netdev - Re: [PATCH v1] Remove zero length skb's when enqueuing new OOB

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: <20240910134451.GD572255@kernel.org>
Date: Tue, 10 Sep 2024 14:44:51 +0100
From: Simon Horman <horms@...nel.org>
To: Rao Shoaib <Rao.Shoaib@...cle.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, kuniyu@...zon.com, netdev@...r.kernel.org
Subject: Re: [PATCH v1] Remove zero length skb's when enqueuing new OOB

On Mon, Sep 09, 2024 at 05:28:54PM -0700, Rao Shoaib wrote:
> 13:03 Recent tests show that AF_UNIX socket code does not handle
> the following sequence properly
> 
> Send OOB
> Read OOB
> Send OOB
> Read (Without OOB flag)
> 
> The last read returns the OOB byte, which is incorrect.
> A following read with OOB flag returns EFAULT, which is also incorrect.
> 
> In AF_UNIX, OOB byte is stored in a single skb, a pointer to the
> skb is stored in the linux socket (oob_skb) and the skb is linked
> in the socket's receive queue. Obviously, there are two refcnts on
> the skb.
> 
> If the byte is read as an OOB, there will be no remaining data and
> regular read frees the skb in managge_oob() and moves to the next skb.
> The bug was that the next skb could be an OOB byte, but the code did
> not check that which resulted in a regular read, receiving the OOB byte.
> 
> This patch adds code check the next skb obtained when a zero
> length skb is freed.
> 
> The patch also adds code to check and remove an skb in front
> of about to be added OOB if it is a zero length skb.
> 
> The cause of the last EFAULT was that the OOB byte had already been read
> by the regular read but oob_skb was not cleared. This resulted in
> __skb_datagram_iter() receiving a zero length skb to copy a byte from.
> So EFAULT was returned.
> 
> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
> Signed-off-by: Rao Shoaib <Rao.Shoaib@...cle.com>

Hi Rao,

This is not a proper review, I will leave that to Iwashima-san and others.

But I would like to note that as a fix for net it needs to be annotated as
such.

	Subject: [PATCH net v1] ...

Unfortunately while the patch applies to net it does not apply to net-next.
But without the above annotation the CI did not know to apply the patch to
net. So the CI can't process this patch.

I suggest posting a v2, targeted at net, after waiting for a review from
Iwashima-san and others.

-- 
pw-bot: cr