linux-kernel - Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts

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]

Date:	Wed, 09 Jun 2010 02:20:01 -0500
From:	Milton Miller <miltonm@....com>
To:	Joe Perches <joe@...ches.com>
Cc:	<linux-kernel@...r.kernel.org>, <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH 00/12] treewide: Remove unnecessary kmalloc casts

On Mon Jun 07 2010 at around 23:53:18 EST, Joe Perches wrote:
> Trivial cleanups
> 
> arch/cris: Remove unnecessary kmalloc casts
> arch/powerpc: Remove unnecessary kmalloc casts
> arch/x86/kernel/tlb_uv.c: Remove unnecessary kmalloc casts
> drivers/gpu/drm: Remove unnecessary kmalloc casts
> drivers/isdn/capi/capidrv.c: Remove unnecessary kmalloc casts
> drivers/media: Remove unnecesary kmalloc casts
> drivers/message/i2o/i20_config.c: Remove unnecessary kmalloc casts
> drivers/parisc/iosapic.c: Remove unnecessary kzalloc cast
> drivers/s390: Remove unnecessary kmalloc casts
> drivers/serial/icom.c: Remove unnecessary kmalloc casts
> drivers/usb/storage/isd200.c: Remove unnecessary kmalloc casts
> fs/ufs/util.c: Remove unnecessary kmalloc casts
> 

Joe,

Thanks for doing cleanups.

However, in this case you are removing casts that, while not necessary
for C, are indeed there for a reason.

Specifically, they are of the form
   type *p;
   <code>
   p = (type *)kmalloc(sizeof(type), ...);

For example, from the powerpc patch:
> goto out;
> }
> - tmp_part = (struct nvram_partition *)
> - kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
> + tmp_part = kmalloc(sizeof(struct nvram_partition), GFP_KERNEL);
> err = -ENOMEM;

The reason they casts are present is to guard against someone changing
the type of p at the top of the function and not changing the type at
the kmalloc.

This was discussed some years ago, possibly around the time kcalloc
got its underflow detection.

Should we create a kmalloc wrapper that enforces this type saftey?
While we have added static analysis tools that can check these things,
and we have slab redzoning options, they tend to be run in batch by
"someone else" or in response to debugging a problem.

There may have been discussion of doing the above vs
   p = kmalloc(sizeof(*p), ...);

I don't remember if it was programmer preference or issues when p is
an array type.

I am not subscribed so I don't have the full list that you sent
these to, and I know some maintainers have already taken some of
the series the last time you posted; could you please augment the
CC list.

thanks,
milton
--
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/