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

Mark Wielaard <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[email protected]

--- Comment #3 from Mark Wielaard <[email protected]> ---
(In reply to mcermak from comment #1)
> Created attachment 187550 [details]
> proposed patch

Looks good with three small comments:

> Declare lsm_list_modules wrappers in priv_syswrap-linux.h and hook it
> for {amd64,arm,arm64,mips64,\ ppc32,ppc64,riscv64,s390x,x86}-linux.

That \ in the middle is odd.

+   if (ML_(safe_to_deref)((__vki_u32 *)ARG2,sizeof(__vki_u32))) {
+      PRE_MEM_READ("lsm_list_modules(size)", ARG2, sizeof(__vki_u32));
+      PRE_MEM_READ("lsm_list_modules(ids)", ARG1, *(__vki_u32 *)ARG2);
+   }

The PRE_MEM_READ of the ARG2 value/pointer itself can be done without checking
whether or not it can be dereferenced, so please do that before the if
statement. The other PRE_MEM_READ does indeed need to be guarded because it
dereferences ARG2.

+POST(sys_lsm_list_modules)
+{
+   POST_MEM_WRITE((Addr)ARG2, sizeof(__vki_u32));
+   POST_MEM_WRITE(ARG1, *(__vki_u32 *)ARG2);
+}

The first POST_MEM_WRITE is correct, the kernel will write out the actual size
used. But also redundant because the value is (should) already (be) defined
when going into the syscall. The value might change, but whether or not it is
defined doesn't.

Looks good to go with those issues addressed or explained.

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

Reply via email to