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

On Thu, May 16, 2019 at 4:43 PM Oleksandr Natalenko
[off-list ref] wrote:

On Thu, May 16, 2019 at 04:20:13PM +0200, Oleksandr Natalenko wrote:

quoted

quoted

[...]

quoted

@@ -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?

Ah, yes, other code like e.g. proc_setgroups_write() uses strncmp()
and then has an extra check to make sure everything trailing is
whitespace.