On 1/10/24 16:32, Linus Torvalds wrote:
On Wed, 10 Jan 2024 at 14:23, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
with this patch in the tree, all sh4 builds fail with ICE.
during RTL pass: final
In file included from fs/namespace.c:11:
fs/namespace.c: In function '__se_sys_listmount':
include/linux/syscalls.h:258:9: internal compiler error: in change_address_1, at emit-rtl.c:2275
We do have those very ugly SYSCALL_DEFINEx() macros, but I'm not
seeing _anything_ that would be odd about the listmount case.
And the "__se_sys" thing in particular is just a fairly trivial wrapper.
It does use that asmlinkage_protect() thing, and it is unquestionably
horrendously ugly (staring too long at <linux/syscalls.h> has been
known to cause madness and despair), but we do that for *every* single
system call and I don't see why the new listmount entry would be
different.
It isn't the syscall. The following hack avoids the problem.
diff --git a/fs/namespace.c b/fs/namespace.c
index ef1fd6829814..28fe2a55bd94 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5070,8 +5070,10 @@ static ssize_t do_listmount(struct mount *first, struct path *orig, u64 mnt_id,
ctr = array_index_nospec(ctr, bufsize);
if (put_user(r->mnt_id_unique, buf + ctr))
return -EFAULT;
+#if 0
if (check_add_overflow(ctr, 1, &ctr))
return -ERANGE;
+#endif
But it isn't check_add_overflow() either. This "helps" as well.
diff --git a/fs/namespace.c b/fs/namespace.c
index ef1fd6829814..b53cb2f13530 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5068,8 +5068,10 @@ static ssize_t do_listmount(struct mount *first, struct path *orig, u64 mnt_id,
if (!is_path_reachable(r, r->mnt.mnt_root, orig))
continue;
ctr = array_index_nospec(ctr, bufsize);
+#if 0
if (put_user(r->mnt_id_unique, buf + ctr))
return -EFAULT;
+#endif
if (check_add_overflow(ctr, 1, &ctr))
return -ERANGE;
Any variance of put_user() with &buf[ctr] or buf + ctr fails
if ctr is a variable and permitted to be != 0. For example,
commenting out the call to check_add_overflow() and starting
the loop with ctr = 1 also triggers the problem, as does replacing
the call to check_add_overflow() with ctr++;. Using a temporary
variable such as in
u64 __user *pbuf;
...
pbuf = buf + ctr;
if (put_user(r->mnt_id_unique, pbuf))
return -EFAULT;
doesn't help either. But this does:
- if (put_user(r->mnt_id_unique, buf + ctr))
+ if (put_user(r->mnt_id_unique, (u32 *)(buf + ctr)))
and "buf + 17" as well as "&buf[17]" work as well. Essentially,
every combination of "buf + ctr" or "&buf[ctr]" fails if buf
is u64* and ctr is a variable.
The following works. Would this be acceptable ?
diff --git a/fs/namespace.c b/fs/namespace.c
index ef1fd6829814..dc0f844205d9 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5068,10 +5068,11 @@ static ssize_t do_listmount(struct mount *first, struct path *orig, u64 mnt_id,
if (!is_path_reachable(r, r->mnt.mnt_root, orig))
continue;
ctr = array_index_nospec(ctr, bufsize);
- if (put_user(r->mnt_id_unique, buf + ctr))
+ if (put_user(r->mnt_id_unique, buf))
return -EFAULT;
if (check_add_overflow(ctr, 1, &ctr))
return -ERANGE;
+ buf++;
}
return ctr;
}
Guenter
