All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Lennart Poettering <mzxreary@0pointer.net>
Cc: Jiri Olsa <olsajiri@gmail.com>,
	bpf@vger.kernel.org, torvalds@linux-foundation.org
Subject: Re: bpf kernel code leaks internal error codes to userspace
Date: Fri, 31 May 2024 09:16:24 +0200	[thread overview]
Message-ID: <Zll5SJcJxvu_yXgt@krava> (raw)
In-Reply-To: <ZliKX5EOU9eWhd2U@gardel-login>

On Thu, May 30, 2024 at 04:17:03PM +0200, Lennart Poettering wrote:
> On Do, 30.05.24 14:18, Jiri Olsa (olsajiri@gmail.com) wrote:
> 
> > > It seems that the bpf code in the kernel sometimes leaks
> > > kernel-internal error codes, i.e. those from include/linux/errno.h
> > > into userspace (as opposed to those from
> > > include/uapi/asm-generic/errno.h which are public userspace facing
> > > API).
> > >
> > > According to the comments from that internal header file: "These
> > > should never be seen by user programs."
> > >
> > > Specifically, this is about ENOTSUPP, which userspace simply cannot
> > > handle, there's no error 524 defined in glibc or anywhere else.
> > >
> > > We ran into this in systemd recently:
> > >
> > > https://github.com/systemd/systemd/issues/32170#issuecomment-2076928761
> > >
> > > (a google search reveals others were hit by this too)
> > >
> > > We commited a work-around for this for now:
> > >
> > > https://github.com/systemd/systemd/pull/33067
> > >
> > > But it really sucks to work around this in userspace, this is a kernel
> > > internal definition after all, conflicting with userspace (where
> > > ENOTSUPP is just an alias for EOPNOTSUPP), hence not really fixable.
> > >
> > > ENOSUPP is kinda useless anyway, since EOPNOTSUPP is pretty much
> > > equally expressive, and something userspace can actually handle.
> > >
> > > Various kernel subsystems have been fixed over the years in similar
> > > situations. For example:
> > >
> > > https://patchwork.kernel.org/project/linux-wireless/patch/20231211085121.3841b71c867d.Idf2ad01d9dfe8d6d6c352bf02deb06e49701ad1d@changeid/
> > >
> > > or
> > >
> > > https://patchwork.kernel.org/project/linux-media/patch/af5b2e8ac6695383111328267a689bcf1c0ecdb1.1702369869.git.sean@mess.org/
> > >
> > > or
> > >
> > > https://patchwork.ozlabs.org/project/linux-mtd/patch/20231129064311.272422-1-acelan.kao@canonical.com/
> > >
> > > I think BPF should really fix that, too.
> >
> > hm, I don't think we can change that, user space already depends
> > on those values and we'd break it with new value
> 
> Are you sure about that? To be able to handle this situation that
> userspace program whose existance you are indicating would have had to
> go the extra mile to literally handle error code 524 that is not known
> to userspace otherwise and handle it. If somebody goes the extra mile
> to do that, what makes you think that they didn't just handle it as
> equivalent to regular EOPNOSTUPP? In systemd at least that's what we
> are doing.

cilium/ebpf [1] library is checking return values just for ENOTSUPP(524)
on multiple places, libbpf has one place to check on that value for
program type detection AFAICS

jirka


[1] https://github.com/cilium/ebpf/

> 
> Also: if various other subsystems (I linked examples from wireless,
> media, mtd above) just fixed this, why not bpf, why is it special in
> this regard?
> 
> AFAICS the man pages of most syscalls just list errnos that *can* be
> returned, but usually doesn't list the precise conditions or makes
> guarantees about it. This specific error is not listed at all on any
> man page for the bpf() syscall, hence are you really sure this is
> actually as set in stone as you think it is?
> 
> I mean, bpf() is still a bit of bleeding edge tech, and people playing
> around with this probably tend to have quite new toolchains even, not
> old, hard to fix code?
> 
> > it's unfortunate, but I don't think we can do much about that,
> > other than enforcing EOPNOTSUPP for new code
> 
> I took the liberty to CC Linus on this:
> 
> Linus, what's the policy if some subsystem by mistake is leaking
> internal kernel error codes (such as ENOTSUP) to userspace? Leave it
> be as is (i.e. error number not defined in include/uapi/, not
> documented, but still returned), or fix it to the closest matching
> public error code (which is probably EOPNOTSUPP in this case)
> accepting a – mild, I would say – compat break?
> 
> [BTW, wouldn't it make sense to add a BUG_ON or so on syscalls that
> return an error number > 133 or so? This kind of issue is quite a
> recurring theme, see the patches above, and such a BUG_ON would
> probably catch 95% of all cases like this.]
> 
> Lennart

  parent reply	other threads:[~2024-05-31  7:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-29 10:08 bpf kernel code leaks internal error codes to userspace Lennart Poettering
2024-05-30 12:18 ` Jiri Olsa
2024-05-30 14:17   ` Lennart Poettering
2024-05-30 17:09     ` Linus Torvalds
2024-05-31  7:16     ` Jiri Olsa [this message]
2024-05-31 17:10       ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zll5SJcJxvu_yXgt@krava \
    --to=olsajiri@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=mzxreary@0pointer.net \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.