Linux RAID subsystem development
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: shli@kernel.org
Cc: linux-raid@vger.kernel.org
Subject: [bug report] raid5: offload stripe handle to workqueue
Date: Thu, 21 Sep 2017 14:45:28 +0300	[thread overview]
Message-ID: <20170921114528.zr6igl744fqd4rx3@mwanda> (raw)

Hello Shaohua Li,

The patch 851c30c9badf: "raid5: offload stripe handle to workqueue"
from Aug 28, 2013, leads to the following static checker warning:

	drivers/md/raid5.c:6658 alloc_thread_groups()
	warn: potential integer overflow from user '232 * cnt'

drivers/md/raid5.c
  6574  static ssize_t
  6575  raid5_store_group_thread_cnt(struct mddev *mddev, const char *page, size_t len)
  6576  {
  6577          struct r5conf *conf;
  6578          unsigned long new;
  6579          int err;
  6580          struct r5worker_group *new_groups, *old_groups;
  6581          int group_cnt, worker_cnt_per_group;
  6582  
  6583          if (len >= PAGE_SIZE)
  6584                  return -EINVAL;
  6585          if (kstrtoul(page, 10, &new))
                                       ^^^^
We get "new" from the user

  6586                  return -EINVAL;
  6587  
  6588          err = mddev_lock(mddev);
  6589          if (err)
  6590                  return err;
  6591          conf = mddev->private;
  6592          if (!conf)
  6593                  err = -ENODEV;
  6594          else if (new != conf->worker_cnt_per_group) {
  6595                  mddev_suspend(mddev);
  6596  
  6597                  old_groups = conf->worker_groups;
  6598                  if (old_groups)
  6599                          flush_workqueue(raid5_wq);
  6600  
  6601                  err = alloc_thread_groups(conf, new,
                                                        ^^^
and pass it to alloc_thread_groups().

  6602                                            &group_cnt, &worker_cnt_per_group,
  6603                                            &new_groups);
  6604                  if (!err) {
  6605                          spin_lock_irq(&conf->device_lock);
  6606                          conf->group_cnt = group_cnt;
  6607                          conf->worker_cnt_per_group = worker_cnt_per_group;
  6608                          conf->worker_groups = new_groups;
  6609                          spin_unlock_irq(&conf->device_lock);
  6610  
  6611                          if (old_groups)
  6612                                  kfree(old_groups[0].workers);
  6613                          kfree(old_groups);
  6614                  }

[ cut ]

  6642  static int alloc_thread_groups(struct r5conf *conf, int cnt,
                                                            ^^^^^^^
"cnt" was unsigned long but now it's an int.

  6643                                 int *group_cnt,
  6644                                 int *worker_cnt_per_group,
  6645                                 struct r5worker_group **worker_groups)
  6646  {
  6647          int i, j, k;
  6648          ssize_t size;
  6649          struct r5worker *workers;
  6650  
  6651          *worker_cnt_per_group = cnt;
  6652          if (cnt == 0) {
  6653                  *group_cnt = 0;
  6654                  *worker_groups = NULL;
  6655                  return 0;
  6656          }
  6657          *group_cnt = num_possible_nodes();
  6658          size = sizeof(struct r5worker) * cnt;
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  6659          workers = kzalloc(size * *group_cnt, GFP_NOIO);
                                  ^^^^^^^^^^^^^^^^^
These multiplications can overflow.

  6660          *worker_groups = kzalloc(sizeof(struct r5worker_group) *
  6661                                  *group_cnt, GFP_NOIO);
  6662          if (!*worker_groups || !workers) {
  6663                  kfree(workers);
  6664                  kfree(*worker_groups);
  6665                  return -ENOMEM;
  6666          }
  6667  
  6668          for (i = 0; i < *group_cnt; i++) {
  6669                  struct r5worker_group *group;

regards,
dan carpenter

                 reply	other threads:[~2017-09-21 11:45 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170921114528.zr6igl744fqd4rx3@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-raid@vger.kernel.org \
    --cc=shli@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox