Re: dm: rename multipath path selector source files to have "dm-ps" prefix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/11/20 9:24 AM, Mike Snitzer wrote:
On Wed, Nov 11 2020 at  6:45am -0500,
Colin Ian King <colin.king@xxxxxxxxxxxxx> wrote:

Hi,

Static analysis on linux-next has detected an initialized variable issue
with the following recent commit:

commit 28784347451fdbf4671ba97018f816041ba2306a
Author: Mike Snitzer <snitzer@xxxxxxxxxx>
Date:   Tue Nov 10 13:41:53 2020 -0500

     dm: rename multipath path selector source files to have "dm-ps" prefix

The analysis is as follows:

  43
static int ioa_add_path(struct path_selector *ps, struct dm_path *path,
  44                        int argc, char **argv, char **error)
  45 {
  46        struct selector *s = ps->context;
  47        struct path_info *pi = NULL;
    1. var_decl: Declaring variable cpu without initializer.

  48        unsigned int cpu;
  49        int ret;
  50
    2. Condition argc != 1, taking false branch.

  51        if (argc != 1) {
  52                *error = "io-affinity ps: invalid number of arguments";
  53                return -EINVAL;
  54        }
  55

    Uninitialized scalar variable (UNINIT)
    3. uninit_use_in_call: Using uninitialized value cpu when calling
__cpu_to_node.

  56        pi = kzalloc_node(sizeof(*pi), GFP_KERNEL, cpu_to_node(cpu));
  57        if (!pi) {
  58                *error = "io-affinity ps: Error allocating path context";
  59                return -ENOMEM;
  60        }

Valid report, but it focuses on a follow-on commit that is just noise.
The commit "dm mpath: add IO affinity path selector" is what is in
quesation, see:
https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.11&id=c3d0a31e609e299836fa6ced28cb9ec06b408181__;!!GqivPVa7Brio!KJegykpnucM-IY4IWXzkUVvJeWoxyfPKk8SAwHrcy8iMldT01JwSwV-Jelxc3x0461yv$

Regardless, Mike Christie, Colin's report has identified a bug.

Please advise on how you'd like to fix ioa_add_path()'s allocation of
'struct path_info'.. pretty sure 'cpu' will default to 0 (on stack)
despite no explicit initialization... so code "works" but does not
do so with a specific cpu allocation affinity.


I meant to drop the kzalloc_node and use kzalloc. I had experimented with allocating structs on specific nodes, but I didn't see much improvement.

Do you prefer I send a patch on top of what's in your branch now, or is that like a staging type of branch where you'll drop my original patch and want me to resubmit the original patch with the fix integrated?

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux