netdev - Re: [PATCH v5 01/11] net: qrtr: ns: validate msglen before ctrl

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

Message-ID: <20250822152108.323af5e5@kernel.org>
Date: Fri, 22 Aug 2025 15:21:08 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Mihai Moldovan <ionic@...ic.de>
Cc: linux-arm-msm@...r.kernel.org, Manivannan Sadhasivam <mani@...nel.org>,
 Denis Kenzior <denkenz@...il.com>, Eric Dumazet <edumazet@...gle.com>,
 Kuniyuki Iwashima <kuniyu@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Willem de Bruijn <willemb@...gle.com>, "David S . Miller"
 <davem@...emloft.net>, Simon Horman <horms@...nel.org>,
 linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v5 01/11] net: qrtr: ns: validate msglen before ctrl_pkt
 use

On Fri, 22 Aug 2025 21:08:47 +0200 Mihai Moldovan wrote:
> >> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> >> index 3de9350cbf30..2bcfe539dc3e 100644
> >> --- a/net/qrtr/ns.c
> >> +++ b/net/qrtr/ns.c
> >> @@ -619,6 +619,9 @@ static void qrtr_ns_worker(struct work_struct *work)
> >>   			break;
> >>   		}
> >>   
> >> +		if ((size_t)msglen < sizeof(*pkt))
> >> +			break;  
> > 
> > why not continue?  
> 
> I don't really know and am not familiar with the QRTR protocol, but here's my 
> best guess:
> 
> Since we're using non-blocking I/O, it doesn't seem to make sense to continue, 
> because the next receive call would just break out anyway once it returns no 
> data at all. Notice that we're also breaking out for -EAGAIN.
> 
> Also, if we somehow got a short read, and we're currently dropping the buffer we 
> just read, any additional data after a subsequent receive would be garbage to us 
> anyway. We'd probably have to keep the old buffer content around and concatenate 
> it with data returned from a new receive call.

Okay, I don't know this proto and driver either. Just reading the
existing code it seemed like it's only breaks if the socket itself
has an error. If the command is not recognized or garbage the loop
will at most print an error and carry on looping..