linux-kernel - Re: [PATCH] SCSI: aacraid: potential integer overflow in aac_get

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] [day] [month] [year] [list]

Message-ID: <CAHrvArRoY5E+TX4Ciy2itiSX-Xq5Z+VvE9TYw+kSRR9wZE6rAA@mail.gmail.com>
Date:	Sat, 3 Dec 2011 14:30:57 -0500
From:	Haogang Chen <haogangchen@...il.com>
To:	Mark Salyzyn <mark_salyzyn@...xyratex.com>
Cc:	aacraid@...ptec.com, JBottomley@...allels.com,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] SCSI: aacraid: potential integer overflow in aac_get_containers()

No, maximum_num_containers is not bounded to MAXIMUM_NUM_CONTAINERS,
it coud be very large. If you look at the code carefully:

if (maximum_num_containers < MAXIMUM_NUM_CONTAINERS)
        maximum_num_containers = MAXIMUM_NUM_CONTAINERS;

MAXIMUM_NUM_CONTAINERS (32) is actually the *minimal* value of
maximum_num_containers, though I'm not sure what's the underlying
logic of this check.


- Haogang



On Fri, Dec 2, 2011 at 11:03 AM, Mark Salyzyn
<mark_salyzyn@...xyratex.com> wrote:
> NAK
>
> I dispute that it is necessary or worth the additional abstraction,
> since MAXIMUM_NUM_CONTAINERS is in the order of 32, and
> sizeof(*fsa_dev_ptr) is in the range of 80 bytes ... There is a LONG
> road to hoe to get to the point of overload!
>
> kcalloc -> __kmalloc(size_t, flags | __GFP_ZERO);
> kzalloc -> kmalloc(size_t, flags | __GFP_ZERO);
> kmalloc -> __kmalloc(size_t, flags);
>
> kcalloc can not allocate a larger entity than kzalloc. But alas it can
> report that the size has exceeded ULONG_MAX so that part of the patch is
> VERY sound.
>
> Sincerely -- Mark Salyzyn
>
> -----Original Message-----
> From: linux-scsi-owner@...r.kernel.org
> [mailto:linux-scsi-owner@...r.kernel.org] On Behalf Of Haogang Chen
> Sent: Wednesday, November 30, 2011 9:30 PM
> To: aacraid@...ptec.com
> Cc: JBottomley@...allels.com; linux-scsi@...r.kernel.org;
> linux-kernel@...r.kernel.org; haogangchen@...il.com
> Subject: [PATCH] SCSI: aacraid: potential integer overflow in
> aac_get_containers()
>
> There is a potential integer overflow in aac_get_containers(). When
> maximum_num_containers is large, the subsequent call to kzalloc() will
> allocate a buffer smaller than expected, which leads to memory
> corruption in the for loop.
>
> The patch replaces kzalloc with kcalloc.
>
> Signed-off-by: Haogang Chen <haogangchen@...il.com>
> ---
>  drivers/scsi/aacraid/aachba.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/aacraid/aachba.c
> b/drivers/scsi/aacraid/aachba.c
> index 409f580..440b84d 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -381,7 +381,7 @@ int aac_get_containers(struct aac_dev *dev)
>
>        if (maximum_num_containers < MAXIMUM_NUM_CONTAINERS)
>                maximum_num_containers = MAXIMUM_NUM_CONTAINERS;
> -       fsa_dev_ptr = kzalloc(sizeof(*fsa_dev_ptr) *
> maximum_num_containers,
> +       fsa_dev_ptr = kcalloc(maximum_num_containers,
> sizeof(*fsa_dev_ptr),
>                        GFP_KERNEL);
>        if (!fsa_dev_ptr)
>                return -ENOMEM;
> --
> 1.7.5.4
> ______________________________________________________________________
> This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
>
> Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
>
> Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
>
> The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
> ______________________________________________________________________
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/