netdev - Re: [PATCH v1 net] netlink: Use copy_to_user() for optval in netlink

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: <e0ec2ca90c10529c597c6c936109e9c73cc55c31.camel@sipsolutions.net>
Date:   Wed, 19 Apr 2023 21:47:29 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Kuniyuki Iwashima <kuniyu@...zon.com>
Cc:     bspencer@...ckberry.com, christophe-h.ricard@...com,
        davem@...emloft.net, dsahern@...il.com, edumazet@...gle.com,
        kaber@...sh.net, kuba@...nel.org, kuni1840@...il.com,
        netdev@...r.kernel.org, pabeni@...hat.com, pablo@...filter.org
Subject: Re: [PATCH v1 net] netlink: Use copy_to_user() for optval in
 netlink_getsockopt().

On Wed, 2023-04-19 at 21:46 +0200, Johannes Berg wrote:
> 
> > > So my 2 cents:
> > >  * I wouldn't remove the checks that the size is at least sizeof(int)
> > >  * I'd - even if it's not strictly backwards compatible - think about
> > >    restricting to *exactly* sizeof(int), which would make the issue
> > >    with the copy_to_user() go away as well (**)
> > >  * if we don't restrict the input length, then need to be much more
> > >    careful about the copy_to_user() I think, but then what if someone
> > >    specifies something really huge as the size?
> > 
> > I'm fine either, but I would prefer the latter using u64 for val and
> > set limit for len as sizeof(u64).
> > 
> 
> That doesn't actually work on big endian,

[snip]

and come to think of it, that also makes the code in your patch now only
work for 'char' or 'short' on little endian, which is again another
tricky gotcha, and also there another argument for not allowing that?

johannes