linux-kernel - Re: [PATCH] USB: gadget: f_mass_storage: fix in fsg_common

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 for Android: free password hash cracker in your pocket

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

Message-id: <op.vck4q0c87p4s8u@pikus>
Date:	Wed, 12 May 2010 11:02:50 +0200
From:	Michał Nazarewicz <m.nazarewicz@...sung.com>
To:	Viral Mehta <Viral.Mehta@...infotech.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Cc:	Kyungmin Park <kyungmin.park@...sung.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] USB: gadget: f_mass_storage: fix in fsg_common_init()
 error recovery

> Michal Nazarewicz [m.nazarewicz@...sung.com]
>> In to places in fsg_common_init() an unconditional call to kfree()
>> on common was performed in error recovery which is not a valid

On Wed, 12 May 2010 10:29:02 +0200, Viral Mehta <Viral.Mehta@...infotech.com> wrote:
> If I am not wrong, common can not be NULL since it was already checked above.
>
> 2685         if (!common) {
> 2686                 common = kzalloc(sizeof *common, GFP_KERNEL);
> 2687                 if (!common)
> 2688                         return ERR_PTR(-ENOMEM);
> 2689                 common->free_storage_on_release = 1;

That is correct but it is not the issue.

fsg_common_init() as a first argument takes a pointer to a fsg_common
structure which, if not NULL, is reused and it is then assumed that
caller is responsible far maintaining allocation and deallocation of
this structure.

The idea is that one can do:

static struct fsg_common fsg_common;
/* ... */
fsg_common_init(&fsg_common, cdev, fsg_config);

or allocate fsg_common structure as a part of a larger structure.  In such
cases kfree() cannot be called on the object.  Which case we are dealing
with is indicated by the common->free_storage_on_release flag.
fsg_common_release() consults it and either calls or does not call krfee().

>> behaviour since fsg_common structure is not always allocated by
>> fsg_common_init().
>>
>> To fix, the calls has been replaced with a goto to a proper error
>> recovery which does the correct thing.

Uh, I've just noticed I forgot about the:

Signed-off-by: Michal Nazarewicz <m.nazarewicz@...sung.com>

-- 
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Michał "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
--
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/