linux-kernel - Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets

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: <20160928112703.GW1876@uranus.lan>
Date:   Wed, 28 Sep 2016 14:27:03 +0300
From:   Cyrill Gorcunov <gorcunov@...il.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        David Ahern <dsa@...ulusnetworks.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, David Miller <davem@...emloft.net>,
        kuznet@....inr.ac.ru, jmorris@...ei.org, yoshfuji@...ux-ipv6.org,
        kaber@...sh.net, avagin@...nvz.org, stephen@...workplumber.org
Subject: Re: [PATCH v5] net: ip, diag -- Add diag interface for raw sockets

On Wed, Sep 28, 2016 at 07:06:26AM -0400, Jamal Hadi Salim wrote:
> > 
> > This structure is uapi, so anyone has complete rights to reference
> > @pad in the userspace programs. Sure it would be more clear to remove
> > the @pad completely, but if we choose so I think it's better to do
> > on top instead and then if someone complain we can easily revert
> > the single trivial commit instead of this big patch.
> 
> I am conflicted.
> A field labelled "pad" does not appear to be valid as "UAPI". It is
> a cosmetic indicator. If you did sizeof() with or without it being
> present the value doesnt change.

I think you miss the point what I'm trying to say: currently end-user
may have reference to this member (for any reason) and his program
will compile and run. If we change the name the compilation procedure
fails and this will break API. Yes, referrning @pad is bad idea for
userspace code, and yes (!) better to simply rename it but lets do
that later, on top, so that if we break something in userspace
we could easily revert the oneline change.

> BTW: There is at least one major structure in inet diag has a hole
> today and doesnt have a padding indicator.
> 
> > If protocol goes over u8 then complete inet_diag_req_v2 structure will
> > have to be reworked becaue @sdiag_protocol is u8 as well. IOW, once
> > someone liftup IPPROTO_MAX > 255, he will notice the problem immediately
> > because diag for such module simply stop working properly.
> > 
> 
> ok.
> 
> cheers,
> jamal
> 

	Cyrill