linux-kernel - [PATCH] Prevent crash on missing sysfs attribute group

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: <20120403081735.78ca3bb3@pluto.restena.lu>
Date:	Tue, 3 Apr 2012 08:17:35 +0200
From:	Bruno Prémont <bonbons@...ux-vserver.org>
To:	Ingo Molnar <mingo@...nel.org>,
	Greg KH <gregkh@...uxfoundation.org>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [PATCH] Prevent crash on missing sysfs attribute group

Prevent kernel from crashing when a device is being registered with sysfs
but has no (aka NULL) group attributes, but warn about it so calling path
can get fixed.

This would warn instead of trying NULL pointer deref like:
 BUG: unable to handle kernel NULL pointer dereference at           (null)
 IP: [<ffffffff81152673>] internal_create_group+0x83/0x1a0
 PGD 0 
 Oops: 0000 [#1] SMP 
 CPU 0 
 Modules linked in:

 Pid: 1, comm: swapper/0 Not tainted 3.4.0-rc1-x86_64 #3 HP ProLiant DL360 G4
 RIP: 0010:[<ffffffff81152673>]  [<ffffffff81152673>] internal_create_group+0x83/0x1a0
 RSP: 0018:ffff88019485fd70  EFLAGS: 00010202
 RAX: 0000000000000001 RBX: 0000000000000000 RCX: 0000000000000001
 RDX: ffff880192e99908 RSI: ffff880192e99630 RDI: ffffffff81a26c60
 RBP: ffff88019485fdc0 R08: 0000000000000000 R09: 0000000000000000
 R10: ffff880192e99908 R11: 0000000000000000 R12: ffffffff81a16a00
 R13: ffff880192e99908 R14: ffffffff81a16900 R15: 0000000000000000
 FS:  0000000000000000(0000) GS:ffff88019bc00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
 CR2: 0000000000000000 CR3: 0000000001a0c000 CR4: 00000000000007f0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
 Process swapper/0 (pid: 1, threadinfo ffff88019485e000, task ffff880194878000)
 Stack:
  ffff88019485fdd0 ffff880192da9d60 0000000000000000 ffff880192e99908
  ffff880192e995d8 0000000000000001 ffffffff81a16a00 ffff880192da9d60
  0000000000000000 0000000000000000 ffff88019485fdd0 ffffffff811527be
 Call Trace:
  [<ffffffff811527be>] sysfs_create_group+0xe/0x10
  [<ffffffff81376ca6>] device_add_groups+0x46/0x80
  [<ffffffff81377d3d>] device_add+0x46d/0x6a0
  ...

Signed-off-by: Bruno Prémont <bonbons@...ux-vserver.org>
---
On Tue, 3 Apr 2012 08:02:52 +0200 Ingo Molnar wrote:
> * Peter Zijlstra <peterz@...radead.org> wrote:
> 
> > On Mon, 2012-04-02 at 23:24 +0200, Peter Zijlstra wrote:
> > > On Mon, 2012-04-02 at 21:34 +0200, Bruno Prémont wrote:
> > > > > >> [    0.996198] Pid: 1, comm: swapper/0 Not tainted 3.4.0-rc1-x86_64 #3 HP ProLiant DL360 G4
> > > 
> > > Is this a netburst based space heater?
> > 
> > In particular, does: https://lkml.org/lkml/2012/3/27/182 , sort it?
> > 
> > Ingo, could you pick that one up and route it Linus wards?
> 
> Will do - but the underlying generic bug should be fixed as 
> well: we must not crash just because some attributes are missing 
> in a rarely used sub-driver ...
> 
> We should WARN_ON(), etc. - but not crash.

Greg, is this ok for you or should the check be moved out to calling
internal_create_group()?

---
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index dd1701c..0040ff2 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -32,7 +32,8 @@ static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
 	struct attribute *const* attr;
 	int error = 0, i;
 
-	for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++) {
+	WARN_ON(!grp->attrs);
+	for (i = 0, attr = grp->attrs; attr && *attr && !error; i++, attr++) {
 		umode_t mode = 0;
 
 		/* in update mode, we're changing the permissions or
--
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/