mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felipe Balbi <balbi@ti.com>, Joel Becker <jlbec@evilplan.org>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] configfs: fix kernel infoleak through user-controlled format string
Date: Tue,  7 Jul 2015 22:28:00 +0800	[thread overview]
Message-ID: <1436279280-28492-2-git-send-email-nicolas.iooss_linux@m4x.org> (raw)
In-Reply-To: <1436279280-28492-1-git-send-email-nicolas.iooss_linux@m4x.org>

Some modules call config_item_init_type_name() and
config_group_init_type_name() with parameter "name" directly controlled
by userspace.  These two functions call config_item_set_name() with this
name used as a format string, which can be used to leak information such
as content of the stack to userspace.

For example, make_netconsole_target() in netconsole module calls
config_item_init_type_name() with the name of a newly-created directory.
This means that the following commands give some unexpected output, with
configfs mounted in /sys/kernel/config/ and on a system with a
configured eth0 ethernet interface:

    # modprobe netconsole
    # mkdir /sys/kernel/config/netconsole/target_%lx
    # echo eth0 > /sys/kernel/config/netconsole/target_%lx/dev_name
    # echo 1 > /sys/kernel/config/netconsole/target_%lx/enabled
    # echo eth0 > /sys/kernel/config/netconsole/target_%lx/dev_name
    # dmesg |tail -n1
    [  142.697668] netconsole: target (target_ffffffffc0ae8080) is
    enabled, disable to update parameters

The directory name is correct but %lx has been interpreted in the
internal item name, displayed here in the error message used by
store_dev_name() in drivers/net/netconsole.c.

To fix this, update every caller of config_item_set_name to use "%s"
when operating on untrusted input.

This issue was found using -Wformat-security gcc flag, once a __printf
attribute has been added to config_item_set_name().

Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
---
 drivers/usb/gadget/configfs.c | 2 +-
 fs/configfs/item.c            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 0495c94a23d7..289e20119fea 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -571,7 +571,7 @@ static struct config_group *function_make(
 	if (IS_ERR(fi))
 		return ERR_CAST(fi);
 
-	ret = config_item_set_name(&fi->group.cg_item, name);
+	ret = config_item_set_name(&fi->group.cg_item, "%s", name);
 	if (ret) {
 		usb_put_function_instance(fi);
 		return ERR_PTR(ret);
diff --git a/fs/configfs/item.c b/fs/configfs/item.c
index 4d6a30e76168..b863a09cd2f1 100644
--- a/fs/configfs/item.c
+++ b/fs/configfs/item.c
@@ -115,7 +115,7 @@ void config_item_init_type_name(struct config_item *item,
 				const char *name,
 				struct config_item_type *type)
 {
-	config_item_set_name(item, name);
+	config_item_set_name(item, "%s", name);
 	item->ci_type = type;
 	config_item_init(item);
 }
@@ -124,7 +124,7 @@ EXPORT_SYMBOL(config_item_init_type_name);
 void config_group_init_type_name(struct config_group *group, const char *name,
 			 struct config_item_type *type)
 {
-	config_item_set_name(&group->cg_item, name);
+	config_item_set_name(&group->cg_item, "%s", name);
 	group->cg_item.ci_type = type;
 	config_group_init(group);
 }
-- 
2.4.5


  reply	other threads:[~2015-07-07 14:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-07 14:27 [PATCH 1/2] include, lib: add __printf attributes to several function prototypes Nicolas Iooss
2015-07-07 14:28 ` Nicolas Iooss [this message]
2015-07-07 17:39   ` [PATCH 2/2] configfs: fix kernel infoleak through user-controlled format string Felipe Balbi
2015-07-07 22:34   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1436279280-28492-2-git-send-email-nicolas.iooss_linux@m4x.org \
    --to=nicolas.iooss_linux@m4x.org \
    --cc=akpm@linux-foundation.org \
    --cc=balbi@ti.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jlbec@evilplan.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®