From: WANG Xuerui <kernel@xen0n.name>
To: linux-api@vger.kernel.org
Cc: Arnd Bergmann <arnd@arndb.de>,
Christian Brauner <brauner@kernel.org>,
Kees Cook <keescook@chromium.org>,
Huacai Chen <chenhuacai@kernel.org>,
Xuefeng Li <lixuefeng@loongson.cn>,
Jianmin Lv <lvjianmin@loongson.cn>,
Xiaotian Wu <wuxiaotian@loongson.cn>,
WANG Rui <wangrui@loongson.cn>,
Miao Wang <shankerwangmiao@gmail.com>,
Icenowy Zheng <uwu@icenowy.me>,
"loongarch@lists.linux.dev" <loongarch@lists.linux.dev>,
linux-arch <linux-arch@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Chromium sandbox on LoongArch and statx -- seccomp deep argument inspection again?
Date: Wed, 21 Feb 2024 14:09:59 +0800 [thread overview]
Message-ID: <599df4a3-47a4-49be-9c81-8e21ea1f988a@xen0n.name> (raw)
Hi,
Recently, we -- community LoongArch porters -- have noticed a problem
where the Chromium sandbox apparently wants to deny statx [^1] so it
could properly inspect arguments after the sandboxed process later falls
back to fstat. The reasoning behind the change was not clear in the
patch; but we found out it's basically because there's currently not a
"fd-only" version of statx, so that the sandbox has no way to ensure the
path argument is empty without being able to peek into the sandboxed
process's memory. For architectures able to do newfstatat though, the
glibc falls back to newfstatat after getting -ENOSYS for statx, then the
respective SIGSYS handler [^2] takes care of inspecting the path
argument, transforming allowed newfstatat's into fstat instead which is
allowed and has the same type of return value.
But, as loongarch is the first architecture to not have fstat nor
newfstatat, the LoongArch glibc does not attempt falling back at all
when it gets -ENOSYS for statx -- and you see the problem there!
Actually, back when the loongarch port was under review, people were
aware of the same problem with sandboxing clone3 [^3], so clone was
eventually kept. Unfortunately it seemed at that time no one had noticed
statx, so besides restoring fstat/newfstatat to loongarch uapi (and
postponing the problem further), it seems inevitable that we would need
to tackle seccomp deep argument inspection; this is obviously a decision
that shouldn't be taken lightly, so I'm posting this to restart the
conversation to figure out a way forward together. We basically could do
one of below:
- just restore fstat and be done with it;
- add a flag to statx so we can do the equivalent of just fstat(fd,
&out) with statx, and ensuring an error happens if path is not empty in
that case;
- tackle the long-standing problem of seccomp deep argument inspection (!).
Obviously, the simplest solution would be to "just restore fstat". But
again, in my opinion this is not quite a solution but a workaround -- we
have good reasons to keep just statx (mainly because its feature set is
a strict superset of those of fstat/newfstatat), and we're not quite in
a hurry to resolve this. Because one of the prerequisites for a new
Chromium port is "inclusion in Debian stable" -- as the loong64 port
[^4] is progressing and the next Debian release would not happen until
2025, we still have time for a more "elegant" solution.
Alternatively, we could also introduce a new flag for statx, maybe named
AT_STATX_NO_PATH or something like that, so that statx would ignore the
path altogether or error on non-empty paths if called with flags
containing AT_EMPTY_PATH | AT_STATX_NO_PATH. This way a seccomp policy
could allow statx calls only with such flags (that are passed via
register and accessible) and maintain the same level of safety as with
fstat. But there is also disadvantage to this approach: the flag would
be useful only for sandboxes, because in other cases there's no need to
avoid reading from &path. This is also more of a workaround to avoid the
deep argument inspection problem.
Lastly, should we decide to go the hardest way, according to a previous
mail [^5] (about clone3) and the LPC 2019 discussion [^6] [^7], we
probably would try the metadata-annotation-based and piece-by-piece
approach, as it's expected to provide the most benefit and involve less
code changes. The implementation, as I surmise, will involve modifying
the generic syscall entrypoint for early copying of user data, and
corresponding changes to seccomp plumbing so this information is
properly exposed. I don't have a roadmap for non-generic-entry arches
right now, and I also haven't started designing the new seccomp ABI for
that. As a matter of fact, members of the LoongArch community (myself
included) are still fresh to this area of expertise, so a bit more of
your feedback will be appreciated.
Thanks to Miao Wang from AOSC for doing much of the investigation.
[^1]: https://chromium-review.googlesource.com/c/chromium/src/+/2823150
[^2]:
https://chromium.googlesource.com/chromium/src/sandbox/+/c085b51940bd/linux/seccomp-bpf-helpers/sigsys_handlers.cc#355
[^3]:
https://lore.kernel.org/linux-arch/20220511211231.GG7074@brightrain.aerifal.cx/
[^4]: https://wiki.debian.org/Ports/loong64
[^5]: https://lwn.net/ml/linux-kernel/201905301122.88FD40B3@keescook/
[^6]: https://lwn.net/Articles/799557/
[^7]:
https://lpc.events/event/4/contributions/560/attachments/397/640/deep-arg-inspection.pdf
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list:https://lore.kernel.org/loongarch/
next reply other threads:[~2024-02-21 6:10 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-21 6:09 WANG Xuerui [this message]
2024-02-21 6:31 ` Chromium sandbox on LoongArch and statx -- seccomp deep argument inspection again? Xi Ruoyao
2024-02-21 10:31 ` Xi Ruoyao
2024-02-21 10:49 ` WANG Xuerui
2024-02-21 12:03 ` Xi Ruoyao
2024-02-24 11:51 ` Huacai Chen
2024-02-25 6:51 ` Icenowy Zheng
2024-02-25 7:32 ` Xi Ruoyao
2024-02-26 6:03 ` Icenowy Zheng
2024-02-26 6:56 ` Arnd Bergmann
2024-02-26 7:09 ` Xi Ruoyao
2024-02-26 9:20 ` Arnd Bergmann
2024-02-26 11:57 ` Xi Ruoyao
2024-02-26 12:57 ` Christian Brauner
2024-02-26 14:33 ` Rich Felker
2024-02-26 13:32 ` Christian Brauner
2024-02-26 13:46 ` Arnd Bergmann
2024-02-26 15:40 ` Christian Brauner
2024-02-26 16:49 ` Xi Ruoyao
2024-02-26 13:46 ` Christian Brauner
2024-02-26 14:00 ` WANG Xuerui
2024-02-26 15:35 ` Christian Brauner
2024-02-26 17:38 ` WANG Xuerui
2024-02-26 8:26 ` Christian Brauner
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=599df4a3-47a4-49be-9c81-8e21ea1f988a@xen0n.name \
--to=kernel@xen0n.name \
--cc=arnd@arndb.de \
--cc=brauner@kernel.org \
--cc=chenhuacai@kernel.org \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lixuefeng@loongson.cn \
--cc=loongarch@lists.linux.dev \
--cc=lvjianmin@loongson.cn \
--cc=shankerwangmiao@gmail.com \
--cc=uwu@icenowy.me \
--cc=wangrui@loongson.cn \
--cc=wuxiaotian@loongson.cn \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).