mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Xi Wang <xi.wang@gmail.com>
To: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Jesper Juhl <jj@chaosbits.net>, Jens Axboe <axboe@kernel.dk>,
	linux-kernel@vger.kernel.org, Matt Mackall <mpm@selenic.com>,
	David Rientjes <rientjes@google.com>
Subject: Re: Uninline kcalloc
Date: Wed, 15 Feb 2012 22:10:59 -0500	[thread overview]
Message-ID: <E307A48D-2146-4052-8A84-E7FA690A6703@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1202151324060.28225@router.home>

On Feb 15, 2012, at 2:34 PM, Christoph Lameter wrote:
> Any allocation larger than MAX_ORDER << PAGE_SHIFT will fail since the
> page allocator cannot serve larger contigous pages.

Yes, any number larger than KMALLOC_MAX_SIZE would do the trick.
The question is whether people would like to adopt

kmalloc(ARRAY_BYTES(n, size), ...);

or

knalloc(n, size, ...);

Besides, I am worried that ARRAY_BYTES is error-prone because you
have to make sure that ARRAY_BYTES is used only in *alloc, not
elsewhere, not in forms like ARRAY_BYTES(n, size) + padding_size,
etc.  If ARRAY_BYTES is mostly used with kmalloc(), and the only
safe form is kmalloc(ARRAY_BYTES(n, size), ...), then adding a new
allocator knalloc(n, size, ...) seems like a simpler choice.

BTW, here goes a partial list of places where the new allocator
could be used to simplify existing checks.

fs/cifs/cifsacl.c:912

        if (num_aces > ULONG_MAX / sizeof(struct cifs_ace *))
                return;                 
        ppace = kmalloc(num_aces * sizeof(struct cifs_ace *),
                        GFP_KERNEL);

fs/cifs/asn1.c:428

        if (size < 2 || size > UINT_MAX/sizeof(unsigned long))
                return 0;

        *oid = kmalloc(size * sizeof(unsigned long), GFP_ATOMIC);

fs/nfs/callback_xdr.c:308

        if (n > ULONG_MAX / sizeof(*args->devs)) {
                status = htonl(NFS4ERR_BADXDR);
                goto out;               
        }

        args->devs = kmalloc(n * sizeof(*args->devs), GFP_KERNEL);

net/ipv4/netfilter/nf_nat_snmp_basic.c:447

        if (size < 2 || size > ULONG_MAX/sizeof(unsigned long))
                return 0;               

        *oid = kmalloc(size * sizeof(unsigned long), GFP_ATOMIC);

drivers/staging/comedi/comedi_fops.c:673

        insns =
            kcalloc(insnlist.n_insns, sizeof(struct comedi_insn), GFP_KERNEL);

I believe there are more places without those checks, where the new
allocator can offer better security.

- xi


  reply	other threads:[~2012-02-16  3:11 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-07 14:11 integer overflows in kernel/relay.c Dan Carpenter
2012-02-08  8:34 ` Jens Axboe
2012-02-08 22:25   ` Andrew Morton
2012-02-09 12:41     ` Jens Axboe
2012-02-09 17:39       ` Andrew Morton
2012-02-09 12:41     ` [PATCH RFC] slab: introduce knalloc/kxnalloc Xi Wang
2012-02-09 13:05       ` Pekka Enberg
2012-02-09 13:19         ` Jens Axboe
2012-02-09 13:26           ` Xi Wang
2012-02-09 13:48             ` [PATCH RFC v2] slab: introduce kmalloc_array Xi Wang
2012-02-09 22:42               ` David Rientjes
2012-02-09 23:08                 ` Andrew Morton
2012-02-09 22:47               ` Jesper Juhl
2012-02-09 23:06                 ` Andrew Morton
2012-02-09 23:43                   ` Joe Perches
2012-02-13 15:08                   ` Xi Wang
2012-02-13 16:01                     ` Christoph Lameter
2012-02-13 19:44                       ` Dan Carpenter
2012-02-13 20:27                         ` Christoph Lameter
2012-02-14  7:20                           ` Dan Carpenter
2012-02-14  7:35                             ` Pekka Enberg
2012-02-14 11:12                             ` Xi Wang
2012-02-14 15:02                             ` Christoph Lameter
2012-02-14 16:30                               ` Xi Wang
2012-02-14 16:34                                 ` Christoph Lameter
2012-02-14 16:43                                   ` Xi Wang
2012-02-14 19:33                                     ` Uninline kcalloc Christoph Lameter
2012-02-14 19:37                                       ` Christoph Lameter
2012-02-14 20:46                                         ` Andrew Morton
2012-02-14 20:50                                         ` Nick Bowler
2012-02-14 21:24                                           ` Christoph Lameter
2012-02-15 20:17                                             ` Nick Bowler
2012-02-15 20:24                                               ` Nick Bowler
2012-02-14 20:45                                       ` Andrew Morton
2012-02-14 20:58                                         ` Pekka Enberg
2012-02-14 21:09                                           ` Christoph Lameter
2012-02-14 21:31                                             ` Pekka Enberg
2012-02-14 21:38                                               ` Christoph Lameter
2012-02-14 21:46                                             ` Xi Wang
2012-02-14 22:08                                               ` Christoph Lameter
2012-02-15 19:14                                                 ` Xi Wang
2012-02-15 19:34                                                   ` Christoph Lameter
2012-02-16  3:10                                                     ` Xi Wang [this message]
2012-02-16 14:51                                                       ` Christoph Lameter
2012-02-16 18:32                                                         ` Xi Wang
2012-02-16 20:47                                                           ` Christoph Lameter
2012-02-10 13:09               ` [PATCH RFC v2] slab: introduce kmalloc_array Alexey Dobriyan
2012-02-10 13:11                 ` Alexey Dobriyan
2012-02-10 13:55                   ` Xi Wang
2012-02-10 13:58                     ` Alexey Dobriyan
2012-02-10 14:09                       ` Xi Wang
2012-02-11 12:19                         ` Alexey Dobriyan
2012-02-12  5:46                           ` Xi Wang
2012-02-09 12:56     ` integer overflows in kernel/relay.c Pekka Enberg
2012-02-09 10:44   ` [patch] relay: prevent integer overflow in relay_open() Dan Carpenter
2012-02-09 11:55     ` walter harms
2012-02-09 12:36       ` Dan Carpenter

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=E307A48D-2146-4052-8A84-E7FA690A6703@gmail.com \
    --to=xi.wang@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=cl@linux.com \
    --cc=dan.carpenter@oracle.com \
    --cc=jj@chaosbits.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpm@selenic.com \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    /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

all inboxes | Powered by JetHome®