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-next>] [day] [month] [year] [list]
Message-ID: <20210317013758.GA134033@roeck-us.net>
Date:   Tue, 16 Mar 2021 18:37:58 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     "menglong8.dong@...il.com" <menglong8.dong@...il.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        "dong.menglong@....com.cn" <dong.menglong@....com.cn>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v4 RESEND net-next] net: socket: use BIT() for MSG_*

On Wed, Mar 17, 2021 at 01:02:51AM +0200, Andy Shevchenko wrote:
> On Wednesday, March 17, 2021, Guenter Roeck <linux@...ck-us.net> wrote:
> 
> > Hi,
> >
> > On Tue, Mar 09, 2021 at 05:51:35PM -0800, menglong8.dong@...il.com wrote:
> > > From: Menglong Dong <dong.menglong@....com.cn>
> > >
> > > The bit mask for MSG_* seems a little confused here. Replace it
> > > with BIT() to make it clear to understand.
> > >
> > > Signed-off-by: Menglong Dong <dong.menglong@....com.cn>
> >
> > I must admit that I am a bit puzzled,
> 
> 
> I have checked the values and don’t see a problem. So, the only difference
> is the type int vs. unsigned long. I think this simply reveals an issue
> somewhere in the code.
> 
The problem is in net/packet/af_packet.c:packet_recvmsg(). This function,
as well as all other rcvmsg functions, is declared as

static int packet_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
                          int flags)

MSG_CMSG_COMPAT (0x80000000) is set in flags, meaning its value is negative.
This is then evaluated in

       if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT|MSG_ERRQUEUE))
                goto out;

If any of those flags is declared as BIT() and thus long, flags is
sign-extended to long. Since it is negative, its upper 32 bits will be set,
the if statement evaluates as true, and the function bails out.

This is relatively easy to fix here with, for example,

        if ((unsigned int)flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT|MSG_ERRQUEUE))
                goto out;

but that is just a hack, and it doesn't solve the real problem:
Each function in struct proto_ops which passes flags passes it as int
(see include/linux/net.h:struct proto_ops). Each such function, if
called with MSG_CMSG_COMPAT set, will fail a match against
~(MSG_anything) if MSG_anything is declared as BIT() or long.

As it turns out, I was kind of lucky to catch the problem: So far I have
seen it only on mips64 kernels with N32 userspace.

Guenter

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ