Received-SPF: pass (google.com: domain of linux-kernel+bounces-135187-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <ZhJTu7qmOtTs9u2c@zeus>
In-Reply-To: <ZhJTu7qmOtTs9u2c@zeus>
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 8 Apr 2024 11:55:54 +0200
Message-ID: <CANn89iJrQevxPFLCj2P=U+XSisYD0jqrUQpa=zWMXTjj5+RriA@mail.gmail.com>
Subject: Re: [PATCH net] nfc: nci: Fix uninit-value in nci_rx_work
To: Ryosuke Yasuoka <ryasuoka@redhat.com>
Cc: krzysztof.kozlowski@linaro.org, davem@davemloft.net, kuba@kernel.org, 
	pabeni@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, 
	syoshida@redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sun, Apr 7, 2024 at 10:05=E2=80=AFAM Ryosuke Yasuoka <ryasuoka@redhat.co=
m> wrote:
>
> syzbot reported the following uninit-value access issue [1]
>
> nci_rx_work() parses received packet from ndev->rx_q. It should be
> checked skb->len is non-zero to verify if it is valid before processing
> the packet. If skb->len is zero but skb->data is not, such packet is
> invalid and should be silently discarded.
>
> Fixes: d24b03535e5e ("nfc: nci: Fix uninit-value in nci_dev_up and nci_nt=
f_packet")
> Reported-and-tested-by: syzbot+d7b4dc6cd50410152534@syzkaller.appspotmail=
com
> Closes: https://syzkaller.appspot.com/bug?extid=3Dd7b4dc6cd50410152534 [1=
]
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
>  net/nfc/nci/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
> index 0d26c8ec9993..b7a020484131 100644
> --- a/net/nfc/nci/core.c
> +++ b/net/nfc/nci/core.c
> @@ -1516,7 +1516,7 @@ static void nci_rx_work(struct work_struct *work)
>                 nfc_send_to_raw_sock(ndev->nfc_dev, skb,
>                                      RAW_PAYLOAD_NCI, NFC_DIRECTION_RX);
>
> -               if (!nci_plen(skb->data)) {
> +               if (!skb->len || !nci_plen(skb->data)) {

#define nci_plen(hdr)           (__u8)((hdr)[2])

So your patch will not help if skb->len is 1 or 2.