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

--- Comment #8 from Mark Wielaard <[email protected]> ---
Looks good. But ...

+PRE(sys_statmount)
+{
+   // int syscall(SYS_statmount, struct mnt_id_req *req,
+   //             struct statmount *smbuf, size_t bufsize,
+   //             unsigned long flags);
+   FUSE_COMPATIBLE_MAY_BLOCK();
+   PRINT("sys_statmount ( %#" FMT_REGWORD "x, %#" FMT_REGWORD "x, %lu,  %#"
FMT_REGWORD "x)", ARG1, ARG2, ARG3, ARG4);
+   PRE_REG_READ4(long, "statmount", struct vki_mnt_id_req *, req, struct
vki_statmount *, smbuf, vki_size_t, bufsize, unsigned long, flags);
+   if (ARG1 == 0) {
+      if (VG_(clo_verbosity) >= 1) {
+         VG_(message)(Vg_UserMsg, "Warning: client syscall sys_statmount: ARG1
is NULL");
+      }
+   } else {
+      struct vki_mnt_id_req *req = (struct vki_mnt_id_req *)(Addr)ARG1;
+      PRE_MEM_READ("statmount(req)", ARG1, req->size);
+   }

I didn't caught this in the previous review, sorry.
But since you are going to dereference req->size the ARG1 check really should
be something like:
struct vki_mnt_id_req *req = (struct vki_mnt_id_req *)(Addr)ARG1;
if (!ML_(safe_to_deref) ((void *)&req->size, sizeof(vki_size_t))) ...

And although I said that if it was NULL (or not safe_to_deref) I would just
want to see a warning I meant from PRE_MEM_READ not really a separate one. So I
would write it as:

struct vki_mnt_id_req *req = (struct vki_mnt_id_req *)(Addr)ARG1;
if (!ML_(safe_to_deref) ((void *)&req->size, sizeof(vki_size_t)))
   PRE_MEM_READ("statmount(req)", ARG1, sizeof(struct vki_mnt_id_req)); // This
will yell and scream because we already know req->size isn't valid.
else
   PRE_MEM_READ("statmount(req)", ARG1, req->size);

+   if (ARG2 == 0) {
+      if (VG_(clo_verbosity) >= 1) {
+         VG_(message)(Vg_UserMsg, "Warning: client syscall sys_statmount: ARG2
is NULL");
+      }
+   } else if (ARG3 > 0) {
+      PRE_MEM_WRITE("statmount(smbuf)", ARG2, ARG3);
+   }

Same here.

+PRE(sys_listmount)
+{
+   // int syscall(SYS_listmount, struct mnt_id_req *req,
+   //             uint64_t *mnt_ids, size_t nr_mnt_ids,
+   //             unsigned long flags);
+   FUSE_COMPATIBLE_MAY_BLOCK();
+   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);
+   if (ARG1 == 0) {
+      if (VG_(clo_verbosity) >= 1) {
+         VG_(message)(Vg_UserMsg, "Warning: client syscall sys_listmount: ARG1
is NULL");
+      }
+   } else {
+      struct vki_mnt_id_req *req = (struct vki_mnt_id_req *)(Addr)ARG1;
+      PRE_MEM_READ("listmount(req)", ARG1, req->size);
+   }

And here.

+   if (ARG2 == 0) {
+      if (VG_(clo_verbosity) >= 1) {
+         VG_(message)(Vg_UserMsg, "Warning: client syscall sys_listmount: ARG2
is NULL");
+      }
+   }
+   else if (ARG3 > 0) {
+      PRE_MEM_WRITE("listmount(mnt_ids)", ARG2, ARG3 * sizeof(vki_uint64_t));
+   }

And I think this can just be an unconditional:
   PRE_MEM_WRITE("listmount(mnt_ids)", ARG2, ARG3 * sizeof(vki_uint64_t));

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

Reply via email to