On Tue, Jul 15, 2025 at 06:10:16PM +0100, Lorenzo Stoakes wrote:
> > For PROCMAP_QUERY, we need priv->mm, but the newly added locked_vma
> > and locked_vma don't need to be persisted between ioctl calls. So we
> > can just add those two fields into a small struct, and for seq_file
> > case have it in priv, but for PROCMAP_QUERY just have it on the stack.
> > The code can be written to accept this struct to maintain the state,
> > which for PROCMAP_QUERY ioctl will be very short-lived on the stack
> > one.
> >
> > Would that work?
>
> Yeah that's a great idea actually, the stack would obviously give us the
> per-query invocation thing. Nice!
>
> I am kicking myself because I jokingly suggested (off-list) that a helper
> struct would be the answer to everything (I do love them) and of
> course... here we are :P
Hm but actually we'd have to invert things I think, what I mean is - since
these fields can be updated at any time by racing threads, we can't have
_anything_ in the priv struct that is mutable.
So instead we should do something like:
struct proc_maps_state {
const struct proc_maps_private *priv;
bool mmap_locked;
struct vm_area_struct *locked_vma;
struct vma_iterator iter;
loff_t last_pos;
};
static long procfs_procmap_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
struct seq_file *seq = file->private_data;
struct proc_maps_private *priv = seq->private;
struct proc_maps_state state = {
.priv = priv,
};
switch (cmd) {
case PROCMAP_QUERY:
return do_procmap_query(state, (void __user *)arg);
default:
return -ENOIOCTLCMD;
}
}
And then we have a stack-based thing with the bits that change and a
read-only pointer to the bits that must remain static. And we can enforce
that with const...
We'd have to move the VMI and last_pos out too to make it const.
Anyway the general idea should work!
