Received-SPF: pass (google.com: domain of linux-kernel+bounces-79995-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <f063e65df92228cac6e57b0c21de6b750cf47e42.camel@icenowy.me>
Subject: Re: Chromium sandbox on LoongArch and statx -- seccomp deep
 argument inspection again?
From: Icenowy Zheng <uwu@icenowy.me>
To: Huacai Chen <chenhuacai@kernel.org>, WANG Xuerui <kernel@xen0n.name>
Cc: linux-api@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>, Christian
 Brauner <brauner@kernel.org>, Kees Cook <keescook@chromium.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>, "loongarch@lists.linux.dev"
 <loongarch@lists.linux.dev>, linux-arch <linux-arch@vger.kernel.org>, Linux
 Kernel Mailing List <linux-kernel@vger.kernel.org>
Date: Sun, 25 Feb 2024 14:51:05 +0800
In-Reply-To: <CAAhV-H4oW70y-2ZSp=b-Ed3A7Jrxfg6xvO8YpjED6To=PF0NwA@mail.gmail.com>
References: <599df4a3-47a4-49be-9c81-8e21ea1f988a@xen0n.name>
	 <CAAhV-H4oW70y-2ZSp=b-Ed3A7Jrxfg6xvO8YpjED6To=PF0NwA@mail.gmail.com>
Organization: Anthon Open-Source Community
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 
Precedence: bulk
MIME-Version: 1.0

=E5=9C=A8 2024-02-24=E6=98=9F=E6=9C=9F=E5=85=AD=E7=9A=84 19:51 +0800=EF=BC=
=8CHuacai Chen=E5=86=99=E9=81=93=EF=BC=9A
> Hi, Xuerui,
>=20
> On Wed, Feb 21, 2024 at 2:10=E2=80=AFPM WANG Xuerui <kernel@xen0n.name>
> wrote:
> >=20
> > Hi,
> >=20
> > 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.
> >=20
> > 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!
> >=20
> > 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:
> >=20
> > - 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 (!).
> From my point of view, I prefer to "restore fstat", because we need
> to
> use the Chrome sandbox everyday (even though it hasn't been upstream
> by now). But I also hope "seccomp deep argument inspection" can be
> solved in the future.

My idea is this problem needs syscalls to be designed with deep
argument inspection in mind; syscalls before this should be considered
as historical error and get fixed by resotring old syscalls.

>=20
>=20
> Huacai
>=20
> >=20
> > 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.
> >=20
> > 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.
> >=20
> > 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.
> >=20
> > Thanks to Miao Wang from AOSC for doing much of the investigation.
> >=20
> > [^1]:
> > https://chromium-review.googlesource.com/c/chromium/src/+/2823150
> > [^2]:
> > https://chromium.googlesource.com/chromium/src/sandbox/+/c085b51940bd/l=
inux/seccomp-bpf-helpers/sigsys_handlers.cc#355
> > [^3]:
> > https://lore.kernel.org/linux-arch/20220511211231.GG7074@brightrain.aer=
ifal.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-a=
rg-inspection.pdf
> >=20
> > --
> > WANG "xen0n" Xuerui
> >=20
> > Linux/LoongArch mailing list:https://lore.kernel.org/loongarch/
> >=20