linux-kernel - Re: [PATCH] sysfs: sysfs_add_one tells you _where

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: <20090212173835.GB20654@ldl.fc.hp.com>
Date:	Thu, 12 Feb 2009 10:38:35 -0700
From:	Alex Chiang <achiang@...com>
To:	Greg KH <gregkh@...e.de>
Cc:	Kay Sievers <kay.sievers@...y.org>, arjan@...ux.intel.com,
	alex.williamson@...com, linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sysfs: sysfs_add_one tells you _where_ the duplicate
	file is

* Greg KH <gregkh@...e.de>:
> On Thu, Feb 12, 2009 at 10:05:23AM +0100, Kay Sievers wrote:
> > On Thu, Feb 12, 2009 at 08:02, Alex Chiang <achiang@...com> wrote:
> > 
> > > sysfs: sysfs_add_one WARNs with full path to duplicate filename
> > >
> > > As a debugging aid, it can be useful to know the full path to a
> > > duplicate file being created in sysfs.
> > 
> > >        ret = __sysfs_add_one(acxt, sd);
> > > -       WARN(ret == -EEXIST, KERN_WARNING "sysfs: duplicate filename '%s' "
> > > -                      "can not be created\n", sd->s_name);
> > > +       if (ret == -EEXIST) {
> > > +               char path[PATH_MAX];
> > 
> > Isn't that 4k on the stack, and a bit large to put there?
> 
> I agree, that's not ok.  Alex, care to redo this?

Here's v4. Adding Arjan to cc because I realized this may cause
problems for kerneloops.org.

If we're displaying a full path now to something like:

	/sys/bus/pci/devices/.../foo

Then the "..." will highly likely be different on every machine
(even though the programming error is still the same).

Is this patch going to cause major heartburn?

Maybe it's more trouble than it's worth... :-/

Thanks.

/ac

From: Alex Chiang <achiang@...com>

sysfs: sysfs_add_one WARNs with full path to duplicate filename

As a debugging aid, it can be useful to know the full path to a
duplicate file being created in sysfs.

We now will display warnings such as:

	sysfs: cannot create duplicate filename '/foo'

when attempting to create multiple files named 'foo' in the sysfs
root, or:

	sysfs: cannot create duplicate filename '/bus/pci/slots/5/foo'

when attempting to create multiple files named 'foo' under a
given directory in sysfs.

The path displayed is always a relative path to sysfs_root. The
leading '/' in the path name refers to the sysfs_root mount
point, and should not be confused with the "real" '/'.

Thanks to Alex Williamson for essentially writing sysfs_pathname.

Cc: Alex Williamson <alex.williamson@...com>
Signed-off-by: Alex Chiang <achiang@...com>
---
 dir.c |   31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)
---
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 82d3b79..ed33361 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -434,6 +434,26 @@ int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
 }
 
 /**
+ *	sysfs_pathname - return full path to sysfs dirent
+ *	@sd: sysfs_dirent whose path we want
+ *	@path: caller allocated buffer
+ *
+ *	Gives the name "/" to the sysfs_root entry; any path returned
+ *	is relative to wherever sysfs is mounted.
+ *
+ *	XXX: does no error checking on @path size
+ */
+static char *sysfs_pathname(struct sysfs_dirent *sd, char *path)
+{
+	if (sd->s_parent) {
+		sysfs_pathname(sd->s_parent, path);
+		strcat(path, "/");
+	}
+	strcat(path, sd->s_name);
+	return path;
+}
+
+/**
  *	sysfs_add_one - add sysfs_dirent to parent
  *	@acxt: addrm context to use
  *	@sd: sysfs_dirent to be added
@@ -458,8 +478,15 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
 	int ret;
 
 	ret = __sysfs_add_one(acxt, sd);
-	WARN(ret == -EEXIST, KERN_WARNING "sysfs: duplicate filename '%s' "
-		       "can not be created\n", sd->s_name);
+	if (ret == -EEXIST) {
+		char *path = kzalloc(PATH_MAX, GFP_KERNEL);
+		WARN(1, KERN_WARNING
+		     "sysfs: cannot create duplicate filename '%s'\n",
+		     strcat(strcat(sysfs_pathname(acxt->parent_sd, path), "/"),
+			    sd->s_name));
+		kfree(path);
+	}
+
 	return ret;
 }
 
--
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/