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

[Simon Horman <horms@kernel.org>]

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@oracle.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