Re: [PATCH RFC 4/5] mm/ksm, proc: introduce remote merge

From: Oleksandr Natalenko
Date: Thu May 16 2019 - 10:45:26 EST


On Thu, May 16, 2019 at 04:20:13PM +0200, Oleksandr Natalenko wrote:
> > [...]
> > > @@ -2960,15 +2962,63 @@ static int proc_stack_depth(struct seq_file *m, struct pid_namespace *ns,
> > > static ssize_t madvise_write(struct file *file, const char __user *buf,
> > > size_t count, loff_t *ppos)
> > > {
> > > + /* For now, only KSM hints are implemented */
> > > +#ifdef CONFIG_KSM
> > > + char buffer[PROC_NUMBUF];
> > > + int behaviour;
> > > struct task_struct *task;
> > > + struct mm_struct *mm;
> > > + int err = 0;
> > > + struct vm_area_struct *vma;
> > > +
> > > + memset(buffer, 0, sizeof(buffer));
> > > + if (count > sizeof(buffer) - 1)
> > > + count = sizeof(buffer) - 1;
> > > + if (copy_from_user(buffer, buf, count))
> > > + return -EFAULT;
> > > +
> > > + if (!memcmp("merge", buffer, min(sizeof("merge")-1, count)))
> >
> > This means that you also match on something like "mergeblah". Just use strcmp().
>
> I agree. Just to make it more interesting I must say that
>
> /sys/kernel/mm/transparent_hugepage/enabled
>
> uses memcmp in the very same way, and thus echoing "alwaysssss" or
> "madviseeee" works perfectly there, and it was like that from the very
> beginning, it seems. Should we fix it, or it became (zomg) a public API?

Actually, maybe, the reason for using memcmp is to handle "echo"
properly: by default it puts a newline character at the end, so if we use
just strcmp, echo should be called with -n, otherwise strcmp won't match
the string.

Huh?

> [...]

--
Best regards,
Oleksandr Natalenko (post-factum)
Senior Software Maintenance Engineer