netdev - Re: [PATCH bpf-next 3/3] xsk: add sock_diag interface for AF

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: <CAJ+HfNgeBHffFo8v6uzNtctT+FARiGUkSb_PG8zRRBibOSo57w@mail.gmail.com>
Date:   Thu, 24 Jan 2019 14:15:05 +0100
From:   Björn Töpel <bjorn.topel@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     ast@...nel.org, Netdev <netdev@...r.kernel.org>,
        Björn Töpel <bjorn.topel@...el.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Magnus Karlsson <magnus.karlsson@...il.com>
Subject: Re: [PATCH bpf-next 3/3] xsk: add sock_diag interface for AF_XDP

Den tors 24 jan. 2019 kl 14:08 skrev Daniel Borkmann <daniel@...earbox.net>:
>
[...]
> >>> +#ifndef _LINUX_XDP_DIAG_H
> >>> +#define _LINUX_XDP_DIAG_H
> >>> +
> >>> +#include <linux/types.h>
> >>> +
> >>> +struct xdp_diag_req {
> >>> +     __u8    sdiag_family;
> >>> +     __u8    sdiag_protocol;
> >>> +     __u16   pad;
> >>
> >> Presumably this one is for future use? Maybe better as '__u16 :16;' to
> >> avoid compile errors if someone tries to zero 'pad' member manually?
> >
> > The "pad" was there simply to have an explicitly named structure hole.
> > I'm not following the bitfield argument. Why does that avoid compiler
> > errors?
>
> Mostly in the sense that an application would explicitly set 'pad = 0'
> whereas pad could later on potentially be renamed and reused otherwise
> (which __u16 :16 would avoid in first place). But looking at other
> *_diag_req structs, it's explicitly named as 'pad' elsewhere already,
> then nevermind, lets have it rather consistent then and keep as is.
>

Ok, thanks for clearing it up. I'll address all your comments, and
also adds a "pad" member in msg for consistency.

> One small thing I still spotted when looking at it again, in function
> xsk_diag_handler_dump() the req is unused.
>

Ah, thank you.


Björn

> Thanks,
> Daniel