Re: [PATCH v4 5/6] add listmount(2) syscall

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux