[bug report] mm: multi-gen LRU: debugfs interface

* [bug report] mm: multi-gen LRU: debugfs interface
@ 2022-08-22 11:21 Dan Carpenter
  2022-08-22 16:49 ` Yu Zhao
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-08-22 11:21 UTC (permalink / raw)
  To: yuzhao; +Cc: linux-mm

Hello Yu Zhao,

The patch e02f70ddcaca: "mm: multi-gen LRU: debugfs interface" from
Aug 15, 2022, leads to the following Smatch static checker warning:

	mm/vmscan.c:5706 lru_gen_seq_write()
	warn: uncapped user index 'cur[end]'

mm/vmscan.c
    5654 static ssize_t lru_gen_seq_write(struct file *file, const char __user *src,
    5655                                  size_t len, loff_t *pos)
    5656 {
    5657         void *buf;
    5658         char *cur, *next;
    5659         unsigned int flags;
    5660         struct blk_plug plug;
    5661         int err = -EINVAL;
    5662         struct scan_control sc = {
    5663                 .may_writepage = true,
    5664                 .may_unmap = true,
    5665                 .may_swap = true,
    5666                 .reclaim_idx = MAX_NR_ZONES - 1,
    5667                 .gfp_mask = GFP_KERNEL,
    5668         };
    5669 
    5670         buf = kvmalloc(len + 1, GFP_KERNEL);
    5671         if (!buf)
    5672                 return -ENOMEM;
    5673 
    5674         if (copy_from_user(buf, src, len)) {
    5675                 kvfree(buf);
    5676                 return -EFAULT;
    5677         }
    5678 
    5679         set_task_reclaim_state(current, &sc.reclaim_state);
    5680         flags = memalloc_noreclaim_save();
    5681         blk_start_plug(&plug);
    5682         if (!set_mm_walk(NULL)) {
    5683                 err = -ENOMEM;
    5684                 goto done;
    5685         }
    5686 
    5687         next = buf;
    5688         next[len] = '\0';
    5689 
    5690         while ((cur = strsep(&next, ",;\n"))) {
    5691                 int n;
    5692                 int end;
    5693                 char cmd;
    5694                 unsigned int memcg_id;
    5695                 unsigned int nid;
    5696                 unsigned long seq;
    5697                 unsigned int swappiness = -1;
    5698                 unsigned long opt = -1;
    5699 
    5700                 cur = skip_spaces(cur);
    5701                 if (!*cur)
    5702                         continue;
    5703 
    5704                 n = sscanf(cur, "%c %u %u %lu %n %u %n %lu %n", &cmd, &memcg_id, &nid,
    5705                            &seq, &end, &swappiness, &end, &opt, &end);
--> 5706                 if (n < 4 || cur[end]) {
                                      ^^^^^^^^
The static checker is correct that "end" comes from the user and it
can be any unsigned int.  This is debugfs code so there is no security
impact.

    5707                         err = -EINVAL;
    5708                         break;
    5709                 }
    5710 
    5711                 err = run_cmd(cmd, memcg_id, nid, seq, &sc, swappiness, opt);
    5712                 if (err)
    5713                         break;
    5714         }
    5715 done:
    5716         clear_mm_walk();
    5717         blk_finish_plug(&plug);
    5718         memalloc_noreclaim_restore(flags);
    5719         set_task_reclaim_state(current, NULL);
    5720 
    5721         kvfree(buf);
    5722 
    5723         return err ? : len;
    5724 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 3+ messages in thread