netdev - Re: [PATCH v2] net: nfc: nc: Add parameter validation for packet data

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: <0be046b6-a48e-45be-9b0e-3922f298b089@linux.dev>
Date: Wed, 17 Sep 2025 17:56:46 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Deepak Sharma <deepak.sharma.472935@...il.com>, krzk@...nel.org
Cc: netdev@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev,
 syzbot+740e04c2a93467a0f8c8@...kaller.appspotmail.com
Subject: Re: [PATCH v2] net: nfc: nc: Add parameter validation for packet data

On 17/09/2025 15:05, Deepak Sharma wrote:
> This is v2 for the original patch, I realized soon after
> sending the patch that I missed the release of skb before
> returning, apologies.

this part shouldn't be in the commit message, it has to go under
strip mark ("---")

> 
> Syzbot reported an uninit-value bug at nci_init_req for commit
> 5aca7966d2a7
> 
> This bug arises due to very limited and poor input validation
> that was done at net/nfc/nci/core.c:1543. This validation only
> validates the skb->len (directly reflects size provided at the
> userspace interface) with the length provided in the buffer
> itself (interpreted as NCI_HEADER). This leads to the processing
> of memory content at the address assuming the correct layout
> per what opcode requires there. This leads to the accesses to
> buffer of `skb_buff->data` which is not assigned anything yet
> 
> Following the same silent drop of packets of invalid sizes, at
> net/nfc/nci/core.c:1543, I have added validation in the
> `nci_nft_packet` which processes NFT packets and silently return
> in case of failure of any validation check
> 
> Possible TODO: because we silently drop the packets, the
> call to `nci_request` will be waiting for completion of request
> and will face timeouts. These timeouts can get excessively logged
> in the dmesg. A proper handling of them may require to export
> `nci_request_cancel` (or propagate error handling from the
> nft packets handlers)
> 
> Reported-by: syzbot+740e04c2a93467a0f8c8@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=740e04c2a93467a0f8c8
> Signed-off-by: Deepak Sharma <deepak.sharma.472935@...il.com>
> ---
>   net/nfc/nci/ntf.c | 42 ++++++++++++++++++++++++++++++++++--------
>   1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
> index a818eff27e6b..f5e03f3ff203 100644
> --- a/net/nfc/nci/ntf.c
> +++ b/net/nfc/nci/ntf.c
> @@ -809,35 +809,61 @@ void nci_ntf_packet(struct nci_dev *ndev, struct sk_buff *skb)
>   
>   	switch (ntf_opcode) {
>   	case NCI_OP_CORE_RESET_NTF:
> -		nci_core_reset_ntf_packet(ndev, skb);
> +		if (skb->len < sizeof(struct nci_core_reset_ntf))
> +			goto end;
> +		else
> +			nci_core_reset_ntf_packet(ndev, skb);
>   		break;

every case here has it's special function. I believe the length check
should be put into these functions and then the return value should
indicate error. That's the actual style of kernel code.

You should also indicate the tree to apply your patch, as this is the
fix, the tree will be net, so the subject should be:
[PATCH net v2] net: nfc: nc: Add parameter validation for packet data

And the Fixes tag should be added to provide some information for
possible backports.