netdev - Re: select(0, ..) is valid ?

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: <20070515104453.f901e91f.akpm@linux-foundation.org>
Date:	Tue, 15 May 2007 10:44:53 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Badari Pulavarty <pbadari@...ibm.com>
Cc:	netdev@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
	Christoph Lameter <clameter@....com>
Subject: Re: select(0, ..) is valid ?

On Tue, 15 May 2007 10:29:18 -0700
Badari Pulavarty <pbadari@...ibm.com> wrote:

> Hi,
> 
> Is select(0, ..) is a valid operation ?

Probably - it becomes an elaborate way of doing a sleep.  Whatever - we
used to permit it without error, so we should continue to do so.

> I see that there is no check to prevent this or return
> success early, without doing any work. Do we need one ?
> 
> slub code is complaining that we are doing kmalloc(0).
> 
> ------------[ cut here ]------------
> Badness at include/linux/slub_def.h:88
> Call Trace:
> [c0000001e4eb7640] [c00000000000e650] .show_stack+0x68/0x1b0
> (unreliable)
> [c0000001e4eb76e0] [c00000000029b854] .report_bug+0x94/0xe8
> [c0000001e4eb7770] [c0000000000219f0] .program_check_exception
> +0x12c/0x568
> [c0000001e4eb77f0] [c000000000004a84] program_check_common+0x104/0x180
> --- Exception: 700 at .get_slab+0x4c/0x234
>     LR = .__kmalloc+0x24/0xc4
> [c0000001e4eb7ae0] [c0000001e4eb7b80] 0xc0000001e4eb7b80 (unreliable)
> [c0000001e4eb7b80] [c0000000000a7ff0] .__kmalloc+0x24/0xc4
> [c0000001e4eb7c10] [c0000000000ea720] .compat_core_sys_select+0x90/0x240
> [c0000001e4eb7d00] [c0000000000ec3a4] .compat_sys_select+0xb0/0x190
> [c0000001e4eb7dc0] [c000000000014944] .ppc32_select+0x14/0x28
> [c0000001e4eb7e30] [c00000000000872c] syscall_exit+0x0/0x40
>

I _think_ we can just do

--- a/fs/compat.c~a
+++ a/fs/compat.c
@@ -1566,9 +1566,13 @@ int compat_core_sys_select(int n, compat
 	 */
 	ret = -ENOMEM;
 	size = FDS_BYTES(n);
-	bits = kmalloc(6 * size, GFP_KERNEL);
-	if (!bits)
-		goto out_nofds;
+	if (likely(size)) {
+		bits = kmalloc(6 * size, GFP_KERNEL);
+		if (!bits)
+			goto out_nofds;
+	} else {
+		bits = NULL;
+	}
 	fds.in      = (unsigned long *)  bits;
 	fds.out     = (unsigned long *) (bits +   size);
 	fds.ex      = (unsigned long *) (bits + 2*size);
_

I mean, if that oopses then I'd be very interested in finding out why.

But I'm starting to suspect that it would be better to permit kmalloc(0) in
slub.  It depends on how many more of these things need fixing.

otoh, a kmalloc(0) could be a sign of some buggy/inefficient/weird code, so
there's some value in forcing us to go look at all the callsites.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html