Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 7 Dec 2023 09:58:13 -0500 | From | Gregory Price <> | Subject | Re: [RFC PATCH 07/11] mm/mempolicy: add userland mempolicy arg structure |
| |
On Thu, Dec 07, 2023 at 08:13:22AM +0100, Arnd Bergmann wrote: > On Thu, Dec 7, 2023, at 01:27, Gregory Price wrote: > > This patch adds the new user-api argument structure intended for > > set_mempolicy2 and mbind2. > > > > struct mpol_args { > > /* Basic mempolicy settings */ > > unsigned short mode; > > unsigned short mode_flags; > > unsigned long *pol_nodes; > > unsigned long pol_maxnodes; > > > > /* get_mempolicy2: policy information (e.g. next interleave node) */ > > int policy_node; > > > > /* get_mempolicy2: memory range policy */ > > unsigned long addr; > > int addr_node; > > > > /* all operations: policy home node */ > > unsigned long home_node; > > > > /* mbind2: address ranges to apply the policy */ > > const struct iovec __user *vec; > > size_t vlen; > > }; > > This is not a great structure layout for a system call ABI, > mostly because it requires adding a compat syscall handler > to be usable from 32-bit tasks. It would be nice if this > could be rewritten in a way that uses only fixed-length > members (__u16, __u32, __aligned_u64), though that does > require the use of u64_to_user_ptr() to replace the pointers > and the reverse in userspace. > > Aside from this, you should avoid holes in the data structure. > On 64-bit architectures, the layout above has holes after > policy_node and after addr_node. > > Arnd
doh, clearly i didn't stop to think about alignment. Good eye. I'll redo this with __u/s members and fix the holes.
Didn't stop to think about compat pointers. I don't think the u64_to_user_ptr pattern is offensive, so i'll make that change. At least I don't see what the other options are beyond compat.
Thanks ~Gregory
| ![\](/images/icornerr.gif) |