Re: [PATCH] fix sys cpumap for > 352 NR_CPUS

[Rusty Russell]

On Thu, 2004-06-03 at 14:25, Paul Jackson wrote:
> Rusty wrote:
> > Then just use -1UL as the arg to scnprintf, if you don't have a real
> > number.  That way the overflow will at least have a chance of detection
> > in the sysfs code, which I think it should check in
> > file.c:fill_read_buffer().  Greg?
> 
> That doesn't make sense.

Then I apologize.

Please allow me to demonstrate with code, which should be clearer.

Name: Fix sysfs Node Cpumap for Large NR_CPUS
Status: Booted on 2.6.7-rc2-bk3
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>

As pointed out by Paul Jackson, sometimes 99 chars is not enough.  We
currently get a page from sysfs: that code should check we don't
haven't overrun it, and for futureproofing, detect problem at
buildtime.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .9869-linux-2.6.7-rc2-bk3/drivers/base/node.c .9869-linux-2.6.7-rc2-bk3.updated/drivers/base/node.c
--- .9869-linux-2.6.7-rc2-bk3/drivers/base/node.c	2004-05-31 09:57:07.000000000 +1000
+++ .9869-linux-2.6.7-rc2-bk3.updated/drivers/base/node.c	2004-06-03 16:18:44.000000000 +1000
@@ -21,9 +21,10 @@ static ssize_t node_read_cpumap(struct s
 	cpumask_t mask = node_dev->cpumap;
 	int len;
 
-	/* FIXME - someone should pass us a buffer size (count) or
-	 * use seq_file or something to avoid buffer overrun risk. */
-	len = cpumask_scnprintf(buf, 99 /* XXX FIXME */, mask);
+	/* 2004/06/03: buf currently PAGE_SIZE, need > 1 char per 4 bits. */
+	BUILD_BUG_ON(NR_CPUS/4 > PAGE_SIZE/2);
+
+	len = cpumask_scnprintf(buf, -1UL, mask);
 	len += sprintf(buf + len, "\n");
 	return len;
 }
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .9869-linux-2.6.7-rc2-bk3/fs/sysfs/file.c .9869-linux-2.6.7-rc2-bk3.updated/fs/sysfs/file.c
--- .9869-linux-2.6.7-rc2-bk3/fs/sysfs/file.c	2004-05-31 09:57:31.000000000 +1000
+++ .9869-linux-2.6.7-rc2-bk3.updated/fs/sysfs/file.c	2004-06-03 16:19:39.000000000 +1000
@@ -89,6 +90,7 @@ static int fill_read_buffer(struct file 
 		return -ENOMEM;
 
 	count = ops->show(kobj,attr,buffer->page);
+	BUG_ON(count > PAGE_SIZE);
 	if (count >= 0)
 		buffer->count = count;
 	else

-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/