https://bugs.kde.org/show_bug.cgi?id=502968

--- Comment #6 from Mark Wielaard <[email protected]> ---
Review part 2:

> +PRE(sys_listmount)
> +{
> +   // int syscall(SYS_listmount, struct mnt_id_req *req,
> +   //             uint64_t *mnt_ids, size_t nr_mnt_ids,
> +   //             unsigned long flags);
> +   PRINT("sys_listmount ( %#" FMT_REGWORD "x, %#" FMT_REGWORD "x, %lu,  %#" 
> FMT_REGWORD "x)", ARG1, ARG2, ARG3, ARG4);
> +   PRE_REG_READ4(long, "listmount", struct vki_mnt_id_req *, req, 
> vki_uint64_t *, mnt_ids, vki_size_t, nr_mnt_ids, unsigned long, flags);

OK.
Like with statmount we might want to use *flags |= SfMayBlock;
or maybe FUSE_COMPATIBLE_MAY_BLOCK();

> +   if (ARG1 != 0) {
> +      PRE_MEM_READ("listmount(req)", ARG1, sizeof(struct vki_mnt_id_req));
> +   }
> +   if ((ARG2 != 0) && (ARG3 > 0)) {
> +      PRE_MEM_WRITE("listmount(mnt_ids)", ARG2, ARG3 * sizeof(vki_uint64_t));
> +   }

As with statmount, are ARG1 and ARG2 allowed to be NULL?
If not then just always do the check.

> +}
> +
> +POST(sys_listmount)
> +{
> +   if ((ARG2 != 0) && (ARG3 > 0)) {
> +      POST_MEM_WRITE(ARG2, ARG3 * sizeof(vki_uint64_t));
> +   }
> +}

Likewise.
And I think it should be POST_MEM_WRITE(ARG2, RES * sizeof(vki_uint64_t));
ARG3 is the maximum, RES is the actual number of mnt_ids put in.

> +struct vki_mnt_id_req {
> +   __vki_u32 size;
> +   __vki_u32 spare;
> +   __vki_u64 mnt_id;
> +   __vki_u64 param;
> +   __vki_u64 mnt_ns_id;
> +};

O. I see what that size field is for. There are multiple versions of
this struct. The second variant is bigger than the first (has the
mnt_ns_id field added). See
https://lore.kernel.org/all/49930bdce29a8367a213eb14c1e68e7e49284f86.1719243756.git.jo...@toxicpanda.com/

So where we do
PRE_MEM_READ("statmount(req)", ARG1, sizeof(struct vki_mnt_id_req));
or
PRE_MEM_READ("listmount(req)", ARG1, sizeof(struct vki_mnt_id_req));

We really should check just the struct size from user space:

struct vki_mnt_id_req *req = (struct vki_mnt_id_req *)(Addr)ARG1;
PRE_MEM_READ("statmount(req)", ARG1, req->size);

> +struct vki_statmount {
> +     __vki_u32 size;         /* Total size, including strings */
> +     __vki_u32 mnt_opts;             /* [str] Mount options of the mount */
> +     __vki_u64 mask;         /* What results were written */
> +     __vki_u32 sb_dev_major; /* Device ID */
> +     __vki_u32 sb_dev_minor;
> +     __vki_u64 sb_magic;             /* ..._SUPER_MAGIC */
> +     __vki_u32 sb_flags;             /* 
> SB_{RDONLY,SYNCHRONOUS,DIRSYNC,LAZYTIME} */
> +     __vki_u32 fs_type;              /* [str] Filesystem type */
> +     __vki_u64 mnt_id;               /* Unique ID of mount */
> +     __vki_u64 mnt_parent_id;        /* Unique ID of parent (for root == 
> mnt_id) */
> +     __vki_u32 mnt_id_old;   /* Reused IDs used in proc/.../mountinfo */
> +     __vki_u32 mnt_parent_id_old;
> +     __vki_u64 mnt_attr;             /* MOUNT_ATTR_... */
> +     __vki_u64 mnt_propagation;      /* MS_{SHARED,SLAVE,PRIVATE,UNBINDABLE} 
> */
> +     __vki_u64 mnt_peer_group;       /* ID of shared peer group */
> +     __vki_u64 mnt_master;   /* Mount receives propagation from this ID */
> +     __vki_u64 propagate_from;       /* Propagation from in current 
> namespace */
> +     __vki_u32 mnt_root;             /* [str] Root of mount relative to root 
> of fs */
> +     __vki_u32 mnt_point;    /* [str] Mountpoint relative to current root */
> +     __vki_u64 mnt_ns_id;    /* ID of the mount namespace */
> +     __vki_u64 __spare2[49];
> +     char str[];             /* Variable size part containing strings */
> +};

OK.

> diff --git a/include/vki/vki-scnums-shared-linux.h 
> b/include/vki/vki-scnums-shared-linux.h
> index 32ef8ac13..85bf9d171 100644
> --- a/include/vki/vki-scnums-shared-linux.h
> +++ b/include/vki/vki-scnums-shared-linux.h
> @@ -56,6 +56,8 @@

>  #define __NR_cachestat               451
>  #define __NR_fchmodat2               452
> +#define __NR_statmount          457
> +#define __NR_listmount          458
>  #define __NR_mseal           462

Looks good.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to