linux-kernel - Re: [syzbot] WARNING in mptcp_sendmsg

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]

Date:   Thu, 23 Sep 2021 17:13:53 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     syzbot <syzbot+263a248eec3e875baa7b@...kaller.appspotmail.com>,
        davem@...emloft.net, kuba@...nel.org, linux-kernel@...r.kernel.org,
        mathew.j.martineau@...ux.intel.com, matthieu.baerts@...sares.net,
        mptcp@...ts.linux.dev, netdev@...r.kernel.org,
        syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] WARNING in mptcp_sendmsg_frag

Hello,

On Thu, 2021-09-23 at 17:37 +0300, Dan Carpenter wrote:
> On Thu, Sep 23, 2021 at 05:19:42PM +0300, Dan Carpenter wrote:
> > On Wed, Sep 22, 2021 at 12:32:56PM +0200, Paolo Abeni wrote:
> > > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > 
> > > The debug code helped a bit. It looks like we have singed/unsigned
> > > comparisons issue
> > 
> > There should be a static checker warning for these.  I have created one
> > in response to your email.  It turns out there are a couple other
> > instances of this bug in the same file.

Thank you!

I was quite suprised the plain compiler did not emit a warn, even with
W=1.

> > net/mptcp/protocol.c:479 mptcp_subflow_could_cleanup() warn: unsigned subtraction: '(null)' use '!='
> 
> I should have checked my output a bit more carefully.  I don't want this
> one to generate a warning.
> 
> > net/mptcp/protocol.c:909 mptcp_frag_can_collapse_to() warn: unsigned subtraction: 'pfrag->size - pfrag->offset' use '!='
> 
> Likely "pfrag->offset" can't be larger than "pfrag->size".  Smatch has
> some code to try track this information but it's not clever enough.

Yes, this looks safe, offset can't be larger than size.

Even the last reported warning looks safe to me: 'info->size_goal -
skb->len', we just check for size_goal being greater then skb->len.

Cheers,

Paolo