linux-kernel - Re: [PATCH 07/25] staging: lustre: libcfs: NUMA support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180416142733.s3f36jdd56rkbfbb@mwanda>
Date:   Mon, 16 Apr 2018 17:27:33 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     James Simmons <jsimmons@...radead.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org,
        Andreas Dilger <andreas.dilger@...el.com>,
        Oleg Drokin <oleg.drokin@...el.com>,
        NeilBrown <neilb@...e.com>,
        Amir Shehata <amir.shehata@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Lustre Development List <lustre-devel@...ts.lustre.org>
Subject: Re: [PATCH 07/25] staging: lustre: libcfs: NUMA support

On Mon, Apr 16, 2018 at 12:09:49AM -0400, James Simmons wrote:
> @@ -114,6 +115,15 @@ struct cfs_cpt_table *
>  	memset(cptab->ctb_cpu2cpt, -1,
>  	       nr_cpu_ids * sizeof(cptab->ctb_cpu2cpt[0]));
>  
> +	cptab->ctb_node2cpt = kvmalloc_array(nr_node_ids,
> +					     sizeof(cptab->ctb_node2cpt[0]),
> +					     GFP_KERNEL);
> +	if (!cptab->ctb_node2cpt)
> +		goto failed;
> +
> +	memset(cptab->ctb_node2cpt, -1,
> +	       nr_node_ids * sizeof(cptab->ctb_node2cpt[0]));
> +
>  	cptab->ctb_parts = kvmalloc_array(ncpt, sizeof(cptab->ctb_parts[0]),
>  					  GFP_KERNEL);
>  	if (!cptab->ctb_parts)


You didn't introduce this, but I was explaining earlier that you should
always be suspicious of code which does "goto failed".  The bug here is
that cptab->ctb_parts is allocated with kvmalloc_array() which
doesn't zero out the memory.  So if we only initialize it part way
because art->cpt_nodemask = kzalloc() fails or something then it's
problem:

    91  void
    92  cfs_cpt_table_free(struct cfs_cpt_table *cptab)
    93  {
    94          int i;
    95  
    96          kvfree(cptab->ctb_cpu2cpt);
    97  
    98          for (i = 0; cptab->ctb_parts && i < cptab->ctb_nparts; i++) {
    99                  struct cfs_cpu_partition *part = &cptab->ctb_parts[i];
   100  
   101                  kfree(part->cpt_nodemask);
                             ^^^^^^^^^^^^^^^^^^^
   102                  free_cpumask_var(part->cpt_cpumask);
                                         ^^^^^^^^^^^^^^^^^
These are uninitialized so it will crash.  It turns out there isn't a
kvcalloc() or kvzalloc_array() function.  We don't seem to have a
vcalloc() either...  Very strange.

   103          }
   104  
   105          kvfree(cptab->ctb_parts);
   106  
   107          kfree(cptab->ctb_nodemask);
   108          free_cpumask_var(cptab->ctb_cpumask);
   109  
   110          kfree(cptab);
   111  }

regards,
dan carpenter