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
| ||
|
Message-ID: <CAEf4BzZDFgDOV9dxfC7g+b9QVQtA4j-_dnqnKOYWPwHbzQ5nJw@mail.gmail.com> Date: Mon, 18 Dec 2023 21:38:43 -0800 From: Andrii Nakryiko <andrii.nakryiko@...il.com> To: Linus Torvalds <torvalds@...uxfoundation.org> Cc: Alexei Starovoitov <alexei.starovoitov@...il.com>, Jakub Kicinski <kuba@...nel.org>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Christian Brauner <brauner@...nel.org>, Network Development <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>, Kernel Team <kernel-team@...com> Subject: Re: pull-request: bpf-next 2023-12-18 On Mon, Dec 18, 2023 at 8:34 PM Andrii Nakryiko <andrii.nakryiko@...il.com> wrote: > > On Mon, Dec 18, 2023 at 7:58 PM Linus Torvalds > <torvalds@...uxfoundation.org> wrote: > > > > On Mon, 18 Dec 2023 at 17:48, Alexei Starovoitov > > <alexei.starovoitov@...il.com> wrote: > > > > > > > > > Point taken. > > > We can do s/__u32 token_fd/__u64 token/ > > > and waste upper 32-bit as flags that indicate that lower 32-bit is an FD > > > or > > > are you ok with __u32 token that is 'fd + 1'. > > > > No, you make it follow the standard pattern that Unix has always had: > > file descriptors are _signed_ integer, and negative means error (or > > special cases). > > > > Now, traditionally a 'fd' is literally just of type "int", but for > > structures it's actually good to make it be a sized entity, so just > > make it be __s32, and make any special cases be actual negative > > numbers. > > > > Because I'll just go out on a limb and say that two billion file > > descriptors is enough for anybody, and if we ever were to hit that > > number, we'll have *way* more serious problems elsewhere long long > > before. And in practice, "int" is 32-bit on all current and > > near-future architectures, so "__s32" really is the same as "int" in > > all practical respects, and making the size explicit is just a good > > idea. > > > > You might want to perhaps pre-reserve a few negative numbers for > > actual special cases, eg "openat()" uses > > > > #define AT_FDCWD -100 > > > > which I don't think is a great example to follow in the details: it > > should have parenthesis, and "100" is a rather odd number to choose, > > but it's certainly an example of a not-fundamentally-broken "not a > > file descriptor, but a special case". > > > > Now, if you have a 'flags' or 'cmd' field for *other* reasons, then > > you can certainly just use one of the flags for "I have a file > > descriptor". But don't do some odd "translate values", and don't add > > 32 bits just for that. > > > > Makes sense. Yes, we do have flags for all commands accepting token > FD, except for one, BPF_BTF_LOAD, but it's trivial to add flags there > as well. I'll prepare a patch. The patch is at [0], thanks. [0] https://patchwork.kernel.org/project/netdevbpf/patch/20231219053150.336991-1-andrii@kernel.org/ > > > That's also a perfectly fine traditional unix use (example: socket > > control messages - "struct cmsghdr" with "cmsg_type = SCM_RIGHTS" in > > unix domain sockets). > > > > But if you don't have some other reason for having a separate flag for > > "I also have a file descriptor you should use", then just make a > > negative number mean "no file descriptor". > > > > It's easy to test for the number being negative, but it's also just > > easy to *not* test for, ie it's also perfectly fine to just do > > something like > > > > struct fd f = fdget(fd); > > > > without ever even bothering to test whether 'fd' is negative or not. > > It is guaranteed to fail for negative numbers and just look exactly > > like the "not open" case, so if you don't care about the difference > > between "invalid" and "not open", then a negative fd also works just > > as-is with no extra code at all. > > > > Linus > > > > Linus
Powered by blists - more mailing lists