* [PATCH 1/8] block: do not pass disk names as format strings
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-06 20:52 ` [PATCH 2/8] kobject: sanitize argument for format string Kees Cook
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook, stable
Disk names may contain arbitrary strings, so they must not be interpreted
as format strings. It seems that only md allows arbitrary strings to be
used for disk names, but this could allow for a local memory corruption
from uid 0 into ring 0.
CVE-2013-2851
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Cc: Jens Axboe <axboe@kernel.dk>
---
block/genhd.c | 2 +-
drivers/block/nbd.c | 3 ++-
drivers/scsi/osd/osd_uld.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 20625ee..cdeb527 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -512,7 +512,7 @@ static void register_disk(struct gendisk *disk)
ddev->parent = disk->driverfs_dev;
- dev_set_name(ddev, disk->disk_name);
+ dev_set_name(ddev, "%s", disk->disk_name);
/* delay uevents, until we scanned partition table */
dev_set_uevent_suppress(ddev, 1);
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 037288e..46b35f7 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -714,7 +714,8 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
else
blk_queue_flush(nbd->disk->queue, 0);
- thread = kthread_create(nbd_thread, nbd, nbd->disk->disk_name);
+ thread = kthread_create(nbd_thread, nbd, "%s",
+ nbd->disk->disk_name);
if (IS_ERR(thread)) {
mutex_lock(&nbd->tx_lock);
return PTR_ERR(thread);
diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 0fab6b5..9d86947 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -485,7 +485,7 @@ static int osd_probe(struct device *dev)
oud->class_dev.class = &osd_uld_class;
oud->class_dev.parent = dev;
oud->class_dev.release = __remove;
- error = dev_set_name(&oud->class_dev, disk->disk_name);
+ error = dev_set_name(&oud->class_dev, "%s", disk->disk_name);
if (error) {
OSD_ERR("dev_set_name failed => %d\n", error);
goto err_put_cdev;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/8] kobject: sanitize argument for format string
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
2013-06-06 20:52 ` [PATCH 1/8] block: do not pass disk names as format strings Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-06 20:52 ` [PATCH 3/8] crypto: " Kees Cook
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook
Unlike kobject_set_name(), the kset_create_and_add() interface does not
provide a way to use format strings, so make sure that the interface
cannot be abused accidentally. It looks like all current callers use
static strings, so there's no existing flaw.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
lib/kobject.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kobject.c b/lib/kobject.c
index b7e29a6..4a1f33d 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -805,7 +805,7 @@ static struct kset *kset_create(const char *name,
kset = kzalloc(sizeof(*kset), GFP_KERNEL);
if (!kset)
return NULL;
- retval = kobject_set_name(&kset->kobj, name);
+ retval = kobject_set_name(&kset->kobj, "%s", name);
if (retval) {
kfree(kset);
return NULL;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/8] crypto: sanitize argument for format string
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
2013-06-06 20:52 ` [PATCH 1/8] block: do not pass disk names as format strings Kees Cook
2013-06-06 20:52 ` [PATCH 2/8] kobject: sanitize argument for format string Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-06 20:52 ` [PATCH 4/8] power: ensure event is not used as " Kees Cook
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook, stable
The template lookup interface does not provide a way to use format
strings, so make sure that the interface cannot be abused accidentally.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
crypto/algapi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 6149a6e..7a1ae87 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -495,7 +495,8 @@ static struct crypto_template *__crypto_lookup_template(const char *name)
struct crypto_template *crypto_lookup_template(const char *name)
{
- return try_then_request_module(__crypto_lookup_template(name), name);
+ return try_then_request_module(__crypto_lookup_template(name), "%s",
+ name);
}
EXPORT_SYMBOL_GPL(crypto_lookup_template);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/8] power: ensure event is not used as format string
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
` (2 preceding siblings ...)
2013-06-06 20:52 ` [PATCH 3/8] crypto: " Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-07 0:39 ` Anton Vorontsov
2013-06-06 20:52 ` [PATCH 5/8] device: avoid format string in dev_set_name Kees Cook
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook, stable
The exposed interface for cm_notify_event() could result in the event
msg string being parsed as a format string. Make sure it is only used
as a literal string.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Cc: Anton Vorontsov <cbou@mail.ru>
Cc: David Woodhouse <dwmw2@infradead.org>
---
drivers/power/charger-manager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index fefc39f..98de1dd 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -450,7 +450,7 @@ static void uevent_notify(struct charger_manager *cm, const char *event)
strncpy(env_str, event, UEVENT_BUF_SIZE);
kobject_uevent(&cm->dev->kobj, KOBJ_CHANGE);
- dev_info(cm->dev, event);
+ dev_info(cm->dev, "%s", event);
}
/**
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/8] power: ensure event is not used as format string
2013-06-06 20:52 ` [PATCH 4/8] power: ensure event is not used as " Kees Cook
@ 2013-06-07 0:39 ` Anton Vorontsov
2013-06-07 1:25 ` [PATCH] power: Add missing newlines, fix a couple of typos, add pr_fmt Joe Perches
0 siblings, 1 reply; 15+ messages in thread
From: Anton Vorontsov @ 2013-06-07 0:39 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Andrew Morton, Greg Kroah-Hartman, Jens Axboe,
David S. Miller, Herbert Xu, David Woodhouse, Karsten Keil,
stable
On Thu, Jun 06, 2013 at 01:52:21PM -0700, Kees Cook wrote:
> The exposed interface for cm_notify_event() could result in the event
> msg string being parsed as a format string. Make sure it is only used
> as a literal string.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@vger.kernel.org
> Cc: Anton Vorontsov <cbou@mail.ru>
> Cc: David Woodhouse <dwmw2@infradead.org>
> ---
Thanks a lot, Kees! This is now applied.
> drivers/power/charger-manager.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
> index fefc39f..98de1dd 100644
> --- a/drivers/power/charger-manager.c
> +++ b/drivers/power/charger-manager.c
> @@ -450,7 +450,7 @@ static void uevent_notify(struct charger_manager *cm, const char *event)
> strncpy(env_str, event, UEVENT_BUF_SIZE);
> kobject_uevent(&cm->dev->kobj, KOBJ_CHANGE);
>
> - dev_info(cm->dev, event);
> + dev_info(cm->dev, "%s", event);
> }
>
> /**
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] power: Add missing newlines, fix a couple of typos, add pr_fmt
2013-06-07 0:39 ` Anton Vorontsov
@ 2013-06-07 1:25 ` Joe Perches
2013-06-07 18:31 ` Kees Cook
0 siblings, 1 reply; 15+ messages in thread
From: Joe Perches @ 2013-06-07 1:25 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Kees Cook, linux-kernel, Andrew Morton, Greg Kroah-Hartman,
Jens Axboe, David S. Miller, Herbert Xu, David Woodhouse,
Karsten Keil, stable
Make sure that dev_<level> calls are newline terminated.
Add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to prefix
all pr_<level> calls with "charger-manager: "
Fix a couple of typos.
Fix formats with terminating n that should be \n.
Coalesce formats for easier grep.
Align arguments to open parenthesis for these dev_<level> calls.
Add missing spaces after coalescing multiple string segments.
Signed-off-by: Joe Perches <joe@perches.com>
---
diff --git a/drivers/power/charger-manager.c b/drivers/power/charger-manager.c
index 98de1dd..ba42029 100644
--- a/drivers/power/charger-manager.c
+++ b/drivers/power/charger-manager.c
@@ -12,6 +12,8 @@
* published by the Free Software Foundation.
**/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include <linux/io.h>
#include <linux/module.h>
#include <linux/irq.h>
@@ -195,8 +197,8 @@ static bool is_charging(struct charger_manager *cm)
cm->charger_stat[i],
POWER_SUPPLY_PROP_ONLINE, &val);
if (ret) {
- dev_warn(cm->dev, "Cannot read ONLINE value from %s.\n",
- cm->desc->psy_charger_stat[i]);
+ dev_warn(cm->dev, "Cannot read ONLINE value from %s\n",
+ cm->desc->psy_charger_stat[i]);
continue;
}
if (val.intval == 0)
@@ -210,8 +212,8 @@ static bool is_charging(struct charger_manager *cm)
cm->charger_stat[i],
POWER_SUPPLY_PROP_STATUS, &val);
if (ret) {
- dev_warn(cm->dev, "Cannot read STATUS value from %s.\n",
- cm->desc->psy_charger_stat[i]);
+ dev_warn(cm->dev, "Cannot read STATUS value from %s\n",
+ cm->desc->psy_charger_stat[i]);
continue;
}
if (val.intval == POWER_SUPPLY_STATUS_FULL ||
@@ -289,7 +291,7 @@ static bool is_polling_required(struct charger_manager *cm)
return is_charging(cm);
default:
dev_warn(cm->dev, "Incorrect polling_mode (%d)\n",
- cm->desc->polling_mode);
+ cm->desc->polling_mode);
}
return false;
@@ -331,9 +333,8 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
err = regulator_enable(desc->charger_regulators[i].consumer);
if (err < 0) {
- dev_warn(cm->dev,
- "Cannot enable %s regulator\n",
- desc->charger_regulators[i].regulator_name);
+ dev_warn(cm->dev, "Cannot enable %s regulator\n",
+ desc->charger_regulators[i].regulator_name);
}
}
} else {
@@ -350,9 +351,8 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
err = regulator_disable(desc->charger_regulators[i].consumer);
if (err < 0) {
- dev_warn(cm->dev,
- "Cannot disable %s regulator\n",
- desc->charger_regulators[i].regulator_name);
+ dev_warn(cm->dev, "Cannot disable %s regulator\n",
+ desc->charger_regulators[i].regulator_name);
}
}
@@ -365,9 +365,8 @@ static int try_charger_enable(struct charger_manager *cm, bool enable)
desc->charger_regulators[i].consumer)) {
regulator_force_disable(
desc->charger_regulators[i].consumer);
- dev_warn(cm->dev,
- "Disable regulator(%s) forcibly.\n",
- desc->charger_regulators[i].regulator_name);
+ dev_warn(cm->dev, "Disable regulator(%s) forcibly\n",
+ desc->charger_regulators[i].regulator_name);
}
}
}
@@ -450,7 +449,7 @@ static void uevent_notify(struct charger_manager *cm, const char *event)
strncpy(env_str, event, UEVENT_BUF_SIZE);
kobject_uevent(&cm->dev->kobj, KOBJ_CHANGE);
- dev_info(cm->dev, "%s", event);
+ dev_info(cm->dev, "%s\n", event);
}
/**
@@ -478,7 +477,7 @@ static void fullbatt_vchk(struct work_struct *work)
err = get_batt_uV(cm, &batt_uV);
if (err) {
- dev_err(cm->dev, "%s: get_batt_uV error(%d).\n", __func__, err);
+ dev_err(cm->dev, "%s: get_batt_uV error(%d)\n", __func__, err);
return;
}
@@ -486,7 +485,7 @@ static void fullbatt_vchk(struct work_struct *work)
if (diff < 0)
return;
- dev_info(cm->dev, "VBATT dropped %duV after full-batt.\n", diff);
+ dev_info(cm->dev, "VBATT dropped %duV after full-batt\n", diff);
if (diff > desc->fullbatt_vchkdrop_uV) {
try_charger_restart(cm);
@@ -519,7 +518,7 @@ static int check_charging_duration(struct charger_manager *cm)
duration = curr - cm->charging_start_time;
if (duration > desc->charging_max_duration_ms) {
- dev_info(cm->dev, "Charging duration exceed %lldms",
+ dev_info(cm->dev, "Charging duration exceed %lldms\n",
desc->charging_max_duration_ms);
uevent_notify(cm, "Discharging");
try_charger_enable(cm, false);
@@ -530,9 +529,9 @@ static int check_charging_duration(struct charger_manager *cm)
if (duration > desc->charging_max_duration_ms &&
is_ext_pwr_online(cm)) {
- dev_info(cm->dev, "DisCharging duration exceed %lldms",
+ dev_info(cm->dev, "Discharging duration exceed %lldms\n",
desc->discharging_max_duration_ms);
- uevent_notify(cm, "Recharing");
+ uevent_notify(cm, "Recharging");
try_charger_enable(cm, true);
ret = true;
}
@@ -579,7 +578,7 @@ static bool _cm_monitor(struct charger_manager *cm)
*/
} else if (!cm->emergency_stop && check_charging_duration(cm)) {
dev_dbg(cm->dev,
- "Charging/Discharging duration is out of range");
+ "Charging/Discharging duration is out of range\n");
/*
* Check dropped voltage of battery. If battery voltage is more
* dropped than fullbatt_vchkdrop_uV after fully charged state,
@@ -595,7 +594,7 @@ static bool _cm_monitor(struct charger_manager *cm)
*/
} else if (!cm->emergency_stop && is_full_charged(cm) &&
cm->charger_enabled) {
- dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged.\n");
+ dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged\n");
uevent_notify(cm, default_event_names[CM_EVENT_BATT_FULL]);
try_charger_enable(cm, false);
@@ -725,7 +724,7 @@ static void fullbatt_handler(struct charger_manager *cm)
cm->fullbatt_vchk_jiffies_at = 1;
out:
- dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged.\n");
+ dev_info(cm->dev, "EVENT_HANDLE: Battery Fully Charged\n");
uevent_notify(cm, default_event_names[CM_EVENT_BATT_FULL]);
}
@@ -972,7 +971,7 @@ static bool cm_setup_timer(void)
mutex_unlock(&cm_list_mtx);
if (wakeup_ms < UINT_MAX && wakeup_ms > 0) {
- pr_info("Charger Manager wakeup timer: %u ms.\n", wakeup_ms);
+ pr_info("Charger Manager wakeup timer: %u ms\n", wakeup_ms);
if (rtc_dev) {
struct rtc_wkalrm tmp;
unsigned long time, now;
@@ -1005,8 +1004,7 @@ static bool cm_setup_timer(void)
ret = false;
}
- pr_info("Waking up after %lu secs.\n",
- time - now);
+ pr_info("Waking up after %lu secs\n", time - now);
rtc_time_to_tm(time, &tmp.time);
rtc_set_alarm(rtc_dev, &tmp);
@@ -1101,7 +1099,7 @@ int setup_charger_manager(struct charger_global_desc *gd)
g_desc = NULL;
if (!gd->rtc_only_wakeup) {
- pr_err("The callback rtc_only_wakeup is not given.\n");
+ pr_err("The callback rtc_only_wakeup is not given\n");
return -EINVAL;
}
@@ -1112,7 +1110,7 @@ int setup_charger_manager(struct charger_global_desc *gd)
/* Retry at probe. RTC may be not registered yet */
}
} else {
- pr_warn("No wakeup timer is given for charger manager."
+ pr_warn("No wakeup timer is given for charger manager. "
"In-suspend monitoring won't work.\n");
}
@@ -1138,13 +1136,13 @@ static void charger_extcon_work(struct work_struct *work)
cable->min_uA, cable->max_uA);
if (ret < 0) {
pr_err("Cannot set current limit of %s (%s)\n",
- cable->charger->regulator_name, cable->name);
+ cable->charger->regulator_name, cable->name);
return;
}
pr_info("Set current limit of %s : %duA ~ %duA\n",
- cable->charger->regulator_name,
- cable->min_uA, cable->max_uA);
+ cable->charger->regulator_name,
+ cable->min_uA, cable->max_uA);
}
try_charger_enable(cable->cm, cable->attached);
@@ -1210,9 +1208,8 @@ static int charger_extcon_init(struct charger_manager *cm,
ret = extcon_register_interest(&cable->extcon_dev,
cable->extcon_name, cable->name, &cable->nb);
if (ret < 0) {
- pr_info("Cannot register extcon_dev for %s(cable: %s).\n",
- cable->extcon_name,
- cable->name);
+ pr_info("Cannot register extcon_dev for %s(cable: %s)\n",
+ cable->extcon_name, cable->name);
ret = -EINVAL;
}
@@ -1243,8 +1240,8 @@ static int charger_manager_register_extcon(struct charger_manager *cm)
charger->consumer = regulator_get(cm->dev,
charger->regulator_name);
if (charger->consumer == NULL) {
- dev_err(cm->dev, "Cannot find charger(%s)n",
- charger->regulator_name);
+ dev_err(cm->dev, "Cannot find charger(%s)\n",
+ charger->regulator_name);
ret = -EINVAL;
goto err;
}
@@ -1255,8 +1252,8 @@ static int charger_manager_register_extcon(struct charger_manager *cm)
ret = charger_extcon_init(cm, cable);
if (ret < 0) {
- dev_err(cm->dev, "Cannot initialize charger(%s)n",
- charger->regulator_name);
+ dev_err(cm->dev, "Cannot initialize charger(%s)\n",
+ charger->regulator_name);
goto err;
}
cable->charger = charger;
@@ -1347,10 +1344,8 @@ static ssize_t charger_externally_control_store(struct device *dev,
}
} else {
dev_warn(cm->dev,
- "'%s' regulator should be controlled "
- "in charger-manager because charger-manager "
- "must need at least one charger for charging\n",
- charger->regulator_name);
+ "'%s' regulator should be controlled in charger-manager because charger-manager must need at least one charger for charging\n",
+ charger->regulator_name);
}
return count;
@@ -1386,8 +1381,6 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
snprintf(buf, 10, "charger.%d", i);
str = kzalloc(sizeof(char) * (strlen(buf) + 1), GFP_KERNEL);
if (!str) {
- dev_err(cm->dev, "Cannot allocate memory: %s\n",
- charger->regulator_name);
ret = -ENOMEM;
goto err;
}
@@ -1423,26 +1416,21 @@ static int charger_manager_register_sysfs(struct charger_manager *cm)
!chargers_externally_control)
chargers_externally_control = 0;
- dev_info(cm->dev, "'%s' regulator's externally_control"
- "is %d\n", charger->regulator_name,
- charger->externally_control);
+ dev_info(cm->dev, "'%s' regulator's externally_control is %d\n",
+ charger->regulator_name, charger->externally_control);
ret = sysfs_create_group(&cm->charger_psy.dev->kobj,
&charger->attr_g);
if (ret < 0) {
- dev_err(cm->dev, "Cannot create sysfs entry"
- "of %s regulator\n",
- charger->regulator_name);
+ dev_err(cm->dev, "Cannot create sysfs entry of %s regulator\n",
+ charger->regulator_name);
ret = -EINVAL;
goto err;
}
}
if (chargers_externally_control) {
- dev_err(cm->dev, "Cannot register regulator because "
- "charger-manager must need at least "
- "one charger for charging battery\n");
-
+ dev_err(cm->dev, "Cannot register regulator because charger-manager must need at least one charger for charging battery\n");
ret = -EINVAL;
goto err;
}
@@ -1463,7 +1451,7 @@ static int charger_manager_probe(struct platform_device *pdev)
rtc_dev = rtc_class_open(g_desc->rtc_name);
if (IS_ERR_OR_NULL(rtc_dev)) {
rtc_dev = NULL;
- dev_err(&pdev->dev, "Cannot get RTC %s.\n",
+ dev_err(&pdev->dev, "Cannot get RTC %s\n",
g_desc->rtc_name);
ret = -ENODEV;
goto err_alloc;
@@ -1471,14 +1459,13 @@ static int charger_manager_probe(struct platform_device *pdev)
}
if (!desc) {
- dev_err(&pdev->dev, "No platform data (desc) found.\n");
+ dev_err(&pdev->dev, "No platform data (desc) found\n");
ret = -ENODEV;
goto err_alloc;
}
cm = kzalloc(sizeof(struct charger_manager), GFP_KERNEL);
if (!cm) {
- dev_err(&pdev->dev, "Cannot allocate memory.\n");
ret = -ENOMEM;
goto err_alloc;
}
@@ -1487,7 +1474,6 @@ static int charger_manager_probe(struct platform_device *pdev)
cm->dev = &pdev->dev;
cm->desc = kmemdup(desc, sizeof(struct charger_desc), GFP_KERNEL);
if (!cm->desc) {
- dev_err(&pdev->dev, "Cannot allocate memory.\n");
ret = -ENOMEM;
goto err_alloc_desc;
}
@@ -1498,33 +1484,28 @@ static int charger_manager_probe(struct platform_device *pdev)
* Users may intentionally ignore those two features.
*/
if (desc->fullbatt_uV == 0) {
- dev_info(&pdev->dev, "Ignoring full-battery voltage threshold"
- " as it is not supplied.");
+ dev_info(&pdev->dev, "Ignoring full-battery voltage threshold as it is not supplied\n");
}
if (!desc->fullbatt_vchkdrop_ms || !desc->fullbatt_vchkdrop_uV) {
- dev_info(&pdev->dev, "Disabling full-battery voltage drop "
- "checking mechanism as it is not supplied.");
+ dev_info(&pdev->dev, "Disabling full-battery voltage drop checking mechanism as it is not supplied\n");
desc->fullbatt_vchkdrop_ms = 0;
desc->fullbatt_vchkdrop_uV = 0;
}
if (desc->fullbatt_soc == 0) {
- dev_info(&pdev->dev, "Ignoring full-battery soc(state of"
- " charge) threshold as it is not"
- " supplied.");
+ dev_info(&pdev->dev, "Ignoring full-battery soc(state of charge) threshold as it is not supplied\n");
}
if (desc->fullbatt_full_capacity == 0) {
- dev_info(&pdev->dev, "Ignoring full-battery full capacity"
- " threshold as it is not supplied.");
+ dev_info(&pdev->dev, "Ignoring full-battery full capacity threshold as it is not supplied\n");
}
if (!desc->charger_regulators || desc->num_charger_regulators < 1) {
ret = -EINVAL;
- dev_err(&pdev->dev, "charger_regulators undefined.\n");
+ dev_err(&pdev->dev, "charger_regulators undefined\n");
goto err_no_charger;
}
if (!desc->psy_charger_stat || !desc->psy_charger_stat[0]) {
- dev_err(&pdev->dev, "No power supply defined.\n");
+ dev_err(&pdev->dev, "No power supply defined\n");
ret = -EINVAL;
goto err_no_charger_stat;
}
@@ -1544,9 +1525,8 @@ static int charger_manager_probe(struct platform_device *pdev)
cm->charger_stat[i] = power_supply_get_by_name(
desc->psy_charger_stat[i]);
if (!cm->charger_stat[i]) {
- dev_err(&pdev->dev, "Cannot find power supply "
- "\"%s\"\n",
- desc->psy_charger_stat[i]);
+ dev_err(&pdev->dev, "Cannot find power supply \"%s\"\n",
+ desc->psy_charger_stat[i]);
ret = -ENODEV;
goto err_chg_stat;
}
@@ -1555,7 +1535,7 @@ static int charger_manager_probe(struct platform_device *pdev)
cm->fuel_gauge = power_supply_get_by_name(desc->psy_fuel_gauge);
if (!cm->fuel_gauge) {
dev_err(&pdev->dev, "Cannot find power supply \"%s\"\n",
- desc->psy_fuel_gauge);
+ desc->psy_fuel_gauge);
ret = -ENODEV;
goto err_chg_stat;
}
@@ -1575,9 +1555,7 @@ static int charger_manager_probe(struct platform_device *pdev)
if (!desc->charging_max_duration_ms ||
!desc->discharging_max_duration_ms) {
- dev_info(&pdev->dev, "Cannot limit charging duration "
- "checking mechanism to prevent overcharge/overheat "
- "and control discharging duration");
+ dev_info(&pdev->dev, "Cannot limit charging duration checking mechanism to prevent overcharge/overheat and control discharging duration\n");
desc->charging_max_duration_ms = 0;
desc->discharging_max_duration_ms = 0;
}
@@ -1598,7 +1576,6 @@ static int charger_manager_probe(struct platform_device *pdev)
NUM_CHARGER_PSY_OPTIONAL),
GFP_KERNEL);
if (!cm->charger_psy.properties) {
- dev_err(&pdev->dev, "Cannot allocate for psy properties.\n");
ret = -ENOMEM;
goto err_chg_stat;
}
@@ -1636,8 +1613,8 @@ static int charger_manager_probe(struct platform_device *pdev)
ret = power_supply_register(NULL, &cm->charger_psy);
if (ret) {
- dev_err(&pdev->dev, "Cannot register charger-manager with"
- " name \"%s\".\n", cm->charger_psy.name);
+ dev_err(&pdev->dev, "Cannot register charger-manager with name \"%s\"\n",
+ cm->charger_psy.name);
goto err_register;
}
@@ -1948,7 +1925,7 @@ void cm_notify_event(struct power_supply *psy, enum cm_event_types type,
uevent_notify(cm, msg ? msg : default_event_names[type]);
break;
default:
- dev_err(cm->dev, "%s type not specified.\n", __func__);
+ dev_err(cm->dev, "%s: type not specified\n", __func__);
break;
}
}
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH] power: Add missing newlines, fix a couple of typos, add pr_fmt
2013-06-07 1:25 ` [PATCH] power: Add missing newlines, fix a couple of typos, add pr_fmt Joe Perches
@ 2013-06-07 18:31 ` Kees Cook
2013-06-09 17:10 ` Anton Vorontsov
0 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2013-06-07 18:31 UTC (permalink / raw)
To: Joe Perches
Cc: Anton Vorontsov, LKML, Andrew Morton, Greg Kroah-Hartman,
Jens Axboe, David S. Miller, Herbert Xu, David Woodhouse,
Karsten Keil, # 3.4.x
On Thu, Jun 6, 2013 at 6:25 PM, Joe Perches <joe@perches.com> wrote:
> Make sure that dev_<level> calls are newline terminated.
> Add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to prefix
> all pr_<level> calls with "charger-manager: "
>
> Fix a couple of typos.
> Fix formats with terminating n that should be \n.
> Coalesce formats for easier grep.
> Align arguments to open parenthesis for these dev_<level> calls.
> Add missing spaces after coalescing multiple string segments.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Looks like good clean-ups.
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] power: Add missing newlines, fix a couple of typos, add pr_fmt
2013-06-07 18:31 ` Kees Cook
@ 2013-06-09 17:10 ` Anton Vorontsov
0 siblings, 0 replies; 15+ messages in thread
From: Anton Vorontsov @ 2013-06-09 17:10 UTC (permalink / raw)
To: Kees Cook
Cc: Joe Perches, LKML, Andrew Morton, Greg Kroah-Hartman, Jens Axboe,
David S. Miller, Herbert Xu, David Woodhouse, Karsten Keil,
# 3.4.x
On Fri, Jun 07, 2013 at 11:31:06AM -0700, Kees Cook wrote:
> On Thu, Jun 6, 2013 at 6:25 PM, Joe Perches <joe@perches.com> wrote:
> > Make sure that dev_<level> calls are newline terminated.
> > Add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt to prefix
> > all pr_<level> calls with "charger-manager: "
> >
> > Fix a couple of typos.
> > Fix formats with terminating n that should be \n.
> > Coalesce formats for easier grep.
> > Align arguments to open parenthesis for these dev_<level> calls.
> > Add missing spaces after coalescing multiple string segments.
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
>
> Looks like good clean-ups.
>
> Acked-by: Kees Cook <keescook@chromium.org>
Applied, thanks!
Anton
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/8] device: avoid format string in dev_set_name
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
` (3 preceding siblings ...)
2013-06-06 20:52 ` [PATCH 4/8] power: ensure event is not used as " Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-07 23:10 ` Greg Kroah-Hartman
2013-06-06 20:52 ` [PATCH 6/8] workqueue: avoid format strings in names Kees Cook
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook
Calling dev_set_name with a single paramter causes it to be handled as
a format string. Many callers are passing potentially dynamic string
content, so use "%s" in those cases to avoid any potential accidents,
including wrappers like device_create*() and bdi_register().
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/base/attribute_container.c | 2 +-
drivers/devfreq/devfreq.c | 2 +-
drivers/extcon/extcon-class.c | 2 +-
drivers/hsi/hsi.c | 2 +-
drivers/ide/ide-cd.c | 2 +-
drivers/ide/ide-gd.c | 2 +-
drivers/ide/ide-probe.c | 4 ++--
drivers/ide/ide-tape.c | 2 +-
drivers/infiniband/core/sysfs.c | 2 +-
drivers/infiniband/hw/qib/qib_file_ops.c | 2 +-
drivers/isdn/mISDN/dsp_pipeline.c | 2 +-
drivers/mtd/mtdcore.c | 2 +-
drivers/platform/x86/wmi.c | 2 +-
drivers/scsi/sd.c | 2 +-
drivers/staging/android/timed_output.c | 2 +-
drivers/staging/dgrp/dgrp_sysfs.c | 2 +-
drivers/uwb/lc-dev.c | 2 +-
drivers/video/backlight/backlight.c | 2 +-
drivers/video/backlight/lcd.c | 2 +-
drivers/video/output.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 2 +-
mm/backing-dev.c | 5 ++---
sound/sound_core.c | 2 +-
23 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/base/attribute_container.c b/drivers/base/attribute_container.c
index d78b204..ecc1929 100644
--- a/drivers/base/attribute_container.c
+++ b/drivers/base/attribute_container.c
@@ -167,7 +167,7 @@ attribute_container_add_device(struct device *dev,
ic->classdev.parent = get_device(dev);
ic->classdev.class = cont->class;
cont->class->dev_release = attribute_container_release;
- dev_set_name(&ic->classdev, dev_name(dev));
+ dev_set_name(&ic->classdev, "%s", dev_name(dev));
if (fn)
fn(cont, dev, &ic->classdev);
else
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 3b36797..af8372f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -477,7 +477,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
GFP_KERNEL);
devfreq->last_stat_updated = jiffies;
- dev_set_name(&devfreq->dev, dev_name(dev));
+ dev_set_name(&devfreq->dev, "%s", dev_name(dev));
err = device_register(&devfreq->dev);
if (err) {
put_device(&devfreq->dev);
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 60adc04..8b77e60 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -620,7 +620,7 @@ int extcon_dev_register(struct extcon_dev *edev, struct device *dev)
edev->dev->class = extcon_class;
edev->dev->release = extcon_dev_release;
- dev_set_name(edev->dev, edev->name ? edev->name : dev_name(dev));
+ dev_set_name(edev->dev, "%s", edev->name ? edev->name : dev_name(dev));
if (edev->max_supported) {
char buf[10];
diff --git a/drivers/hsi/hsi.c b/drivers/hsi/hsi.c
index 833dd1a..66d4458 100644
--- a/drivers/hsi/hsi.c
+++ b/drivers/hsi/hsi.c
@@ -75,7 +75,7 @@ static void hsi_new_client(struct hsi_port *port, struct hsi_board_info *info)
cl->device.bus = &hsi_bus_type;
cl->device.parent = &port->device;
cl->device.release = hsi_client_release;
- dev_set_name(&cl->device, info->name);
+ dev_set_name(&cl->device, "%s", info->name);
cl->device.platform_data = info->platform_data;
if (info->archdata)
cl->device.archdata = *info->archdata;
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 2ff6204..0b510ba 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1756,7 +1756,7 @@ static int ide_cd_probe(ide_drive_t *drive)
info->dev.parent = &drive->gendev;
info->dev.release = ide_cd_release;
- dev_set_name(&info->dev, dev_name(&drive->gendev));
+ dev_set_name(&info->dev, "%s", dev_name(&drive->gendev));
if (device_register(&info->dev))
goto out_free_disk;
diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c
index de86631..838996a 100644
--- a/drivers/ide/ide-gd.c
+++ b/drivers/ide/ide-gd.c
@@ -392,7 +392,7 @@ static int ide_gd_probe(ide_drive_t *drive)
idkp->dev.parent = &drive->gendev;
idkp->dev.release = ide_disk_release;
- dev_set_name(&idkp->dev, dev_name(&drive->gendev));
+ dev_set_name(&idkp->dev, "%s", dev_name(&drive->gendev));
if (device_register(&idkp->dev))
goto out_free_disk;
diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index 068cef0..2a744a9 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -545,7 +545,7 @@ static int ide_register_port(ide_hwif_t *hwif)
int ret;
/* register with global device tree */
- dev_set_name(&hwif->gendev, hwif->name);
+ dev_set_name(&hwif->gendev, "%s", hwif->name);
dev_set_drvdata(&hwif->gendev, hwif);
if (hwif->gendev.parent == NULL)
hwif->gendev.parent = hwif->dev;
@@ -559,7 +559,7 @@ static int ide_register_port(ide_hwif_t *hwif)
}
hwif->portdev = device_create(ide_port_class, &hwif->gendev,
- MKDEV(0, 0), hwif, hwif->name);
+ MKDEV(0, 0), hwif, "%s", hwif->name);
if (IS_ERR(hwif->portdev)) {
ret = PTR_ERR(hwif->portdev);
device_unregister(&hwif->gendev);
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index c6c574b..1793aea 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -1985,7 +1985,7 @@ static int ide_tape_probe(ide_drive_t *drive)
tape->dev.parent = &drive->gendev;
tape->dev.release = ide_tape_release;
- dev_set_name(&tape->dev, dev_name(&drive->gendev));
+ dev_set_name(&tape->dev, "%s", dev_name(&drive->gendev));
if (device_register(&tape->dev))
goto out_free_disk;
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 246fdc1..99904f7 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -813,7 +813,7 @@ int ib_device_register_sysfs(struct ib_device *device,
class_dev->class = &ib_class;
class_dev->parent = device->dma_device;
- dev_set_name(class_dev, device->name);
+ dev_set_name(class_dev, "%s", device->name);
dev_set_drvdata(class_dev, device);
INIT_LIST_HEAD(&device->port_list);
diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index b56c942..9dd0bc8 100644
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -2208,7 +2208,7 @@ int qib_cdev_init(int minor, const char *name,
goto err_cdev;
}
- device = device_create(qib_class, NULL, dev, NULL, name);
+ device = device_create(qib_class, NULL, dev, NULL, "%s", name);
if (!IS_ERR(device))
goto done;
ret = PTR_ERR(device);
diff --git a/drivers/isdn/mISDN/dsp_pipeline.c b/drivers/isdn/mISDN/dsp_pipeline.c
index 88305c9..8b1a66c 100644
--- a/drivers/isdn/mISDN/dsp_pipeline.c
+++ b/drivers/isdn/mISDN/dsp_pipeline.c
@@ -102,7 +102,7 @@ int mISDN_dsp_element_register(struct mISDN_dsp_element *elem)
entry->dev.class = elements_class;
entry->dev.release = mISDN_dsp_dev_release;
dev_set_drvdata(&entry->dev, elem);
- dev_set_name(&entry->dev, elem->name);
+ dev_set_name(&entry->dev, "%s", elem->name);
ret = device_register(&entry->dev);
if (ret) {
printk(KERN_ERR "%s: failed to register %s\n",
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c400c57..048c823 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1151,7 +1151,7 @@ static int __init mtd_bdi_init(struct backing_dev_info *bdi, const char *name)
ret = bdi_init(bdi);
if (!ret)
- ret = bdi_register(bdi, NULL, name);
+ ret = bdi_register(bdi, NULL, "%s", name);
if (ret)
bdi_destroy(bdi);
diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index e4ac38a..b13344c 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -743,7 +743,7 @@ static int wmi_create_device(const struct guid_block *gblock,
wblock->dev.class = &wmi_class;
wmi_gtoa(gblock->guid, guid_string);
- dev_set_name(&wblock->dev, guid_string);
+ dev_set_name(&wblock->dev, "%s", guid_string);
dev_set_drvdata(&wblock->dev, wblock);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index c1c5552..8fa3d0b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2931,7 +2931,7 @@ static int sd_probe(struct device *dev)
device_initialize(&sdkp->dev);
sdkp->dev.parent = dev;
sdkp->dev.class = &sd_disk_class;
- dev_set_name(&sdkp->dev, dev_name(dev));
+ dev_set_name(&sdkp->dev, "%s", dev_name(dev));
if (device_add(&sdkp->dev))
goto out_free_index;
diff --git a/drivers/staging/android/timed_output.c b/drivers/staging/android/timed_output.c
index ec9e2ae..ee3a57f 100644
--- a/drivers/staging/android/timed_output.c
+++ b/drivers/staging/android/timed_output.c
@@ -78,7 +78,7 @@ int timed_output_dev_register(struct timed_output_dev *tdev)
tdev->index = atomic_inc_return(&device_count);
tdev->dev = device_create(timed_output_class, NULL,
- MKDEV(0, tdev->index), NULL, tdev->name);
+ MKDEV(0, tdev->index), NULL, "%s", tdev->name);
if (IS_ERR(tdev->dev))
return PTR_ERR(tdev->dev);
diff --git a/drivers/staging/dgrp/dgrp_sysfs.c b/drivers/staging/dgrp/dgrp_sysfs.c
index 7d1b36d..8cee9c8 100644
--- a/drivers/staging/dgrp/dgrp_sysfs.c
+++ b/drivers/staging/dgrp/dgrp_sysfs.c
@@ -273,7 +273,7 @@ void dgrp_create_node_class_sysfs_files(struct nd_struct *nd)
sprintf(name, "node%ld", nd->nd_major);
nd->nd_class_dev = device_create(dgrp_class, dgrp_class_nodes_dev,
- MKDEV(0, nd->nd_major), NULL, name);
+ MKDEV(0, nd->nd_major), NULL, "%s", name);
ret = sysfs_create_group(&nd->nd_class_dev->kobj,
&dgrp_node_attribute_group);
diff --git a/drivers/uwb/lc-dev.c b/drivers/uwb/lc-dev.c
index 5241f1d..9209eaf 100644
--- a/drivers/uwb/lc-dev.c
+++ b/drivers/uwb/lc-dev.c
@@ -440,7 +440,7 @@ void uwbd_dev_onair(struct uwb_rc *rc, struct uwb_beca_e *bce)
uwb_dev_init(uwb_dev); /* This sets refcnt to one, we own it */
uwb_dev->mac_addr = *bce->mac_addr;
uwb_dev->dev_addr = bce->dev_addr;
- dev_set_name(&uwb_dev->dev, macbuf);
+ dev_set_name(&uwb_dev->dev, "%s", macbuf);
result = uwb_dev_add(uwb_dev, &rc->uwb_dev.dev, rc);
if (result < 0) {
dev_err(dev, "new device %s: cannot instantiate device\n",
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index c74e7aa..e3c2790 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -304,7 +304,7 @@ struct backlight_device *backlight_device_register(const char *name,
new_bd->dev.class = backlight_class;
new_bd->dev.parent = parent;
new_bd->dev.release = bl_device_release;
- dev_set_name(&new_bd->dev, name);
+ dev_set_name(&new_bd->dev, "%s", name);
dev_set_drvdata(&new_bd->dev, devdata);
/* Set default properties */
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 34fb6bd..3649fd9 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -219,7 +219,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
new_ld->dev.class = lcd_class;
new_ld->dev.parent = parent;
new_ld->dev.release = lcd_device_release;
- dev_set_name(&new_ld->dev, name);
+ dev_set_name(&new_ld->dev, "%s", name);
dev_set_drvdata(&new_ld->dev, devdata);
rc = device_register(&new_ld->dev);
diff --git a/drivers/video/output.c b/drivers/video/output.c
index 0d6f2cd..6285b97 100644
--- a/drivers/video/output.c
+++ b/drivers/video/output.c
@@ -97,7 +97,7 @@ struct output_device *video_output_register(const char *name,
new_dev->props = op;
new_dev->dev.class = &video_output_class;
new_dev->dev.parent = dev;
- dev_set_name(&new_dev->dev, name);
+ dev_set_name(&new_dev->dev, "%s", name);
dev_set_drvdata(&new_dev->dev, devdata);
ret_code = device_register(&new_dev->dev);
if (ret_code) {
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 56cfaaa..8c9db16 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -447,7 +447,7 @@ int xenbus_probe_node(struct xen_bus_type *bus,
if (err)
goto fail;
- dev_set_name(&xendev->dev, devname);
+ dev_set_name(&xendev->dev, "%s", devname);
/* Register with generic device framework. */
err = device_register(&xendev->dev);
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 5025174..c18e5fd 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -515,7 +515,6 @@ EXPORT_SYMBOL(bdi_destroy);
int bdi_setup_and_register(struct backing_dev_info *bdi, char *name,
unsigned int cap)
{
- char tmp[32];
int err;
bdi->name = name;
@@ -524,8 +523,8 @@ int bdi_setup_and_register(struct backing_dev_info *bdi, char *name,
if (err)
return err;
- sprintf(tmp, "%.28s%s", name, "-%d");
- err = bdi_register(bdi, NULL, tmp, atomic_long_inc_return(&bdi_seq));
+ err = bdi_register(bdi, NULL, "%.28s-%d", name,
+ atomic_long_inc_return(&bdi_seq));
if (err) {
bdi_destroy(bdi);
return err;
diff --git a/sound/sound_core.c b/sound/sound_core.c
index 359753f..45759f4 100644
--- a/sound/sound_core.c
+++ b/sound/sound_core.c
@@ -292,7 +292,7 @@ retry:
}
device_create(sound_class, dev, MKDEV(SOUND_MAJOR, s->unit_minor),
- NULL, s->name+6);
+ NULL, "%s", s->name+6);
return s->unit_minor;
fail:
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/8] device: avoid format string in dev_set_name
2013-06-06 20:52 ` [PATCH 5/8] device: avoid format string in dev_set_name Kees Cook
@ 2013-06-07 23:10 ` Greg Kroah-Hartman
0 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2013-06-07 23:10 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Andrew Morton, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil
On Thu, Jun 06, 2013 at 01:52:22PM -0700, Kees Cook wrote:
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -515,7 +515,6 @@ EXPORT_SYMBOL(bdi_destroy);
> int bdi_setup_and_register(struct backing_dev_info *bdi, char *name,
> unsigned int cap)
> {
> - char tmp[32];
> int err;
>
> bdi->name = name;
> @@ -524,8 +523,8 @@ int bdi_setup_and_register(struct backing_dev_info *bdi, char *name,
> if (err)
> return err;
>
> - sprintf(tmp, "%.28s%s", name, "-%d");
> - err = bdi_register(bdi, NULL, tmp, atomic_long_inc_return(&bdi_seq));
> + err = bdi_register(bdi, NULL, "%.28s-%d", name,
> + atomic_long_inc_return(&bdi_seq));
> if (err) {
> bdi_destroy(bdi);
> return err;
This chunk causes the following build warning to now show up:
mm/backing-dev.c: In function ‘bdi_setup_and_register’:
mm/backing-dev.c:527:7: warning: format ‘%d’ expects argument of type ‘int’, but argument 5 has type ‘long int’ [-Wformat]
Care to fix it up and resend it?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/8] workqueue: avoid format strings in names
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
` (4 preceding siblings ...)
2013-06-06 20:52 ` [PATCH 5/8] device: avoid format string in dev_set_name Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-06 20:52 ` [PATCH 7/8] kthread: avoid parsing names as format strings Kees Cook
2013-06-06 20:52 ` [PATCH 8/8] isdn: clean up debug format string usage Kees Cook
7 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook
For the workqueue creation interfaces that do not expect format strings,
make sure they cannot accidently be parsed that way. Additionally, clean
up calls made with a single parameter that would be handled as a format
string. Many callers are passing potentially dynamic string content,
so use "%s" in those cases to avoid any potential accidents.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
crypto/pcrypt.c | 4 ++--
drivers/media/pci/cx18/cx18-driver.c | 2 +-
drivers/message/i2o/driver.c | 4 ++--
drivers/net/wireless/rt2x00/rt2x00dev.c | 2 +-
drivers/net/wireless/rtlwifi/base.c | 2 +-
drivers/pci/hotplug/pciehp_hpc.c | 4 +---
drivers/pci/hotplug/shpchp_core.c | 3 +--
drivers/scsi/be2iscsi/be_main.c | 2 +-
drivers/scsi/qla4xxx/ql4_os.c | 4 ++--
drivers/scsi/scsi_transport_fc.c | 6 +++---
include/linux/workqueue.h | 7 ++++---
net/bluetooth/hci_core.c | 9 ++++-----
net/mac80211/main.c | 2 +-
13 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index b2c99dc..f8c920c 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -455,8 +455,8 @@ static int pcrypt_init_padata(struct padata_pcrypt *pcrypt,
get_online_cpus();
- pcrypt->wq = alloc_workqueue(name,
- WQ_MEM_RECLAIM | WQ_CPU_INTENSIVE, 1);
+ pcrypt->wq = alloc_workqueue("%s", WQ_MEM_RECLAIM | WQ_CPU_INTENSIVE,
+ 1, name);
if (!pcrypt->wq)
goto err;
diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
index 67b61cf..004d8ac 100644
--- a/drivers/media/pci/cx18/cx18-driver.c
+++ b/drivers/media/pci/cx18/cx18-driver.c
@@ -695,7 +695,7 @@ static int cx18_create_in_workq(struct cx18 *cx)
{
snprintf(cx->in_workq_name, sizeof(cx->in_workq_name), "%s-in",
cx->v4l2_dev.name);
- cx->in_work_queue = alloc_ordered_workqueue(cx->in_workq_name, 0);
+ cx->in_work_queue = alloc_ordered_workqueue("%s", 0, cx->in_workq_name);
if (cx->in_work_queue == NULL) {
CX18_ERR("Unable to create incoming mailbox handler thread\n");
return -ENOMEM;
diff --git a/drivers/message/i2o/driver.c b/drivers/message/i2o/driver.c
index 8a5b2d8..813eaa3 100644
--- a/drivers/message/i2o/driver.c
+++ b/drivers/message/i2o/driver.c
@@ -84,8 +84,8 @@ int i2o_driver_register(struct i2o_driver *drv)
osm_debug("Register driver %s\n", drv->name);
if (drv->event) {
- drv->event_queue = alloc_workqueue(drv->name,
- WQ_MEM_RECLAIM, 1);
+ drv->event_queue = alloc_workqueue("%s", WQ_MEM_RECLAIM, 1,
+ drv->name);
if (!drv->event_queue) {
osm_err("Could not initialize event queue for driver "
"%s\n", drv->name);
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 90dc143..c8b9ef0 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -1321,7 +1321,7 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
* Initialize work.
*/
rt2x00dev->workqueue =
- alloc_ordered_workqueue(wiphy_name(rt2x00dev->hw->wiphy), 0);
+ alloc_ordered_workqueue("%s", 0, wiphy_name(rt2x00dev->hw->wiphy));
if (!rt2x00dev->workqueue) {
retval = -ENOMEM;
goto exit;
diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
index af59dd5..a5f2231 100644
--- a/drivers/net/wireless/rtlwifi/base.c
+++ b/drivers/net/wireless/rtlwifi/base.c
@@ -380,7 +380,7 @@ static void _rtl_init_deferred_work(struct ieee80211_hw *hw)
/* <2> work queue */
rtlpriv->works.hw = hw;
- rtlpriv->works.rtl_wq = alloc_workqueue(rtlpriv->cfg->name, 0, 0);
+ rtlpriv->works.rtl_wq = alloc_workqueue("%s", 0, 0, rtlpriv->cfg->name);
INIT_DELAYED_WORK(&rtlpriv->works.watchdog_wq,
(void *)rtl_watchdog_wq_callback);
INIT_DELAYED_WORK(&rtlpriv->works.ips_nic_off_wq,
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 5127f3f..b225573 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -773,14 +773,12 @@ static void pcie_shutdown_notification(struct controller *ctrl)
static int pcie_init_slot(struct controller *ctrl)
{
struct slot *slot;
- char name[32];
slot = kzalloc(sizeof(*slot), GFP_KERNEL);
if (!slot)
return -ENOMEM;
- snprintf(name, sizeof(name), "pciehp-%u", PSN(ctrl));
- slot->wq = alloc_workqueue(name, 0, 0);
+ slot->wq = alloc_workqueue("pciehp-%u", 0, 0, PSN(ctrl));
if (!slot->wq)
goto abort;
diff --git a/drivers/pci/hotplug/shpchp_core.c b/drivers/pci/hotplug/shpchp_core.c
index 3100c52..d3f757d 100644
--- a/drivers/pci/hotplug/shpchp_core.c
+++ b/drivers/pci/hotplug/shpchp_core.c
@@ -128,8 +128,7 @@ static int init_slots(struct controller *ctrl)
slot->hpc_ops = ctrl->hpc_ops;
slot->number = ctrl->first_slot + (ctrl->slot_num_inc * i);
- snprintf(name, sizeof(name), "shpchp-%d", slot->number);
- slot->wq = alloc_workqueue(name, 0, 0);
+ slot->wq = alloc_workqueue("shpchp-%d", 0, 0, slot->number);
if (!slot->wq) {
retval = -ENOMEM;
goto error_info;
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index d24a286..a1f5ac7 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -4996,7 +4996,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
snprintf(phba->wq_name, sizeof(phba->wq_name), "beiscsi_%02x_wq",
phba->shost->host_no);
- phba->wq = alloc_workqueue(phba->wq_name, WQ_MEM_RECLAIM, 1);
+ phba->wq = alloc_workqueue("%s", WQ_MEM_RECLAIM, 1, phba->wq_name);
if (!phba->wq) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
"BM_%d : beiscsi_dev_probe-"
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 4d231c1..b246b3c 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -7060,8 +7060,8 @@ skip_retry_init:
}
INIT_WORK(&ha->dpc_work, qla4xxx_do_dpc);
- sprintf(buf, "qla4xxx_%lu_task", ha->host_no);
- ha->task_wq = alloc_workqueue(buf, WQ_MEM_RECLAIM, 1);
+ ha->task_wq = alloc_workqueue("qla4xxx_%lu_task", WQ_MEM_RECLAIM, 1,
+ ha->host_no);
if (!ha->task_wq) {
ql4_printk(KERN_WARNING, ha, "Unable to start task thread!\n");
ret = -ENODEV;
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index e106c27..4628fd5 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -435,7 +435,7 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev,
snprintf(fc_host->work_q_name, sizeof(fc_host->work_q_name),
"fc_wq_%d", shost->host_no);
- fc_host->work_q = alloc_workqueue(fc_host->work_q_name, 0, 0);
+ fc_host->work_q = alloc_workqueue("%s", 0, 0, fc_host->work_q_name);
if (!fc_host->work_q)
return -ENOMEM;
@@ -443,8 +443,8 @@ static int fc_host_setup(struct transport_container *tc, struct device *dev,
snprintf(fc_host->devloss_work_q_name,
sizeof(fc_host->devloss_work_q_name),
"fc_dl_%d", shost->host_no);
- fc_host->devloss_work_q =
- alloc_workqueue(fc_host->devloss_work_q_name, 0, 0);
+ fc_host->devloss_work_q = alloc_workqueue("%s", 0, 0,
+ fc_host->devloss_work_q_name);
if (!fc_host->devloss_work_q) {
destroy_workqueue(fc_host->work_q);
fc_host->work_q = NULL;
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 623488f..018c9e7 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -410,11 +410,12 @@ __alloc_workqueue_key(const char *fmt, unsigned int flags, int max_active,
alloc_workqueue(fmt, WQ_UNBOUND | __WQ_ORDERED | (flags), 1, ##args)
#define create_workqueue(name) \
- alloc_workqueue((name), WQ_MEM_RECLAIM, 1)
+ alloc_workqueue("%s", WQ_MEM_RECLAIM, 1, (name))
#define create_freezable_workqueue(name) \
- alloc_workqueue((name), WQ_FREEZABLE | WQ_UNBOUND | WQ_MEM_RECLAIM, 1)
+ alloc_workqueue("%s", WQ_FREEZABLE | WQ_UNBOUND | WQ_MEM_RECLAIM, \
+ 1, (name))
#define create_singlethread_workqueue(name) \
- alloc_workqueue((name), WQ_UNBOUND | WQ_MEM_RECLAIM, 1)
+ alloc_workqueue("%s", WQ_UNBOUND | WQ_MEM_RECLAIM, 1, (name))
extern void destroy_workqueue(struct workqueue_struct *wq);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 33843c5..1bd6844 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2202,16 +2202,15 @@ int hci_register_dev(struct hci_dev *hdev)
list_add(&hdev->list, &hci_dev_list);
write_unlock(&hci_dev_list_lock);
- hdev->workqueue = alloc_workqueue(hdev->name, WQ_HIGHPRI | WQ_UNBOUND |
- WQ_MEM_RECLAIM, 1);
+ hdev->workqueue = alloc_workqueue("%s", WQ_HIGHPRI | WQ_UNBOUND |
+ WQ_MEM_RECLAIM, 1, hdev->name);
if (!hdev->workqueue) {
error = -ENOMEM;
goto err;
}
- hdev->req_workqueue = alloc_workqueue(hdev->name,
- WQ_HIGHPRI | WQ_UNBOUND |
- WQ_MEM_RECLAIM, 1);
+ hdev->req_workqueue = alloc_workqueue("%s", WQ_HIGHPRI | WQ_UNBOUND |
+ WQ_MEM_RECLAIM, 1, hdev->name);
if (!hdev->req_workqueue) {
destroy_workqueue(hdev->workqueue);
error = -ENOMEM;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 8a7bfc4..8eae74a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -921,7 +921,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
hw->queues = IEEE80211_MAX_QUEUES;
local->workqueue =
- alloc_ordered_workqueue(wiphy_name(local->hw.wiphy), 0);
+ alloc_ordered_workqueue("%s", 0, wiphy_name(local->hw.wiphy));
if (!local->workqueue) {
result = -ENOMEM;
goto fail_workqueue;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/8] kthread: avoid parsing names as format strings
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
` (5 preceding siblings ...)
2013-06-06 20:52 ` [PATCH 6/8] workqueue: avoid format strings in names Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-06 20:52 ` [PATCH 8/8] isdn: clean up debug format string usage Kees Cook
7 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook
Calling kthread_run with a single name parameter causes it to be handled
as a format string. Many callers are passing potentially dynamic string
content, so use "%s" in those cases to avoid any potential accidents.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/block/aoe/aoecmd.c | 2 +-
drivers/block/mtip32xx/mtip32xx.c | 3 ++-
drivers/block/xen-blkback/xenbus.c | 2 +-
drivers/hwmon/adt7470.c | 2 +-
drivers/media/i2c/tvaudio.c | 3 ++-
drivers/media/pci/ivtv/ivtv-driver.c | 2 +-
drivers/media/platform/vivi.c | 3 ++-
drivers/mtd/ubi/build.c | 2 +-
drivers/net/wireless/airo.c | 3 ++-
drivers/scsi/aacraid/commctrl.c | 3 ++-
drivers/scsi/aacraid/commsup.c | 3 ++-
drivers/spi/spi.c | 2 +-
drivers/staging/rtl8712/os_intfs.c | 2 +-
drivers/usb/atm/usbatm.c | 5 +++--
fs/lockd/svc.c | 2 +-
fs/nfs/callback.c | 5 ++---
fs/nfs/nfs4state.c | 2 +-
kernel/rcutree.c | 2 +-
net/sunrpc/svc.c | 2 +-
19 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index fc803ec..b75c7db 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -1340,7 +1340,7 @@ aoe_ktstart(struct ktstate *k)
struct task_struct *task;
init_completion(&k->rendez);
- task = kthread_run(kthread, k, k->name);
+ task = kthread_run(kthread, k, "%s", k->name);
if (task == NULL || IS_ERR(task))
return -ENOMEM;
k->task = task;
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 847107e..81ce4c0 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -4085,7 +4085,8 @@ skip_create_disk:
start_service_thread:
sprintf(thd_name, "mtip_svc_thd_%02d", index);
dd->mtip_svc_handler = kthread_create_on_node(mtip_service_thread,
- dd, dd->numa_node, thd_name);
+ dd, dd->numa_node, "%s",
+ thd_name);
if (IS_ERR(dd->mtip_svc_handler)) {
dev_err(&dd->pdev->dev, "service thread failed to start\n");
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 8bfd1bc..04608a6 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -93,7 +93,7 @@ static void xen_update_blkif_status(struct xen_blkif *blkif)
}
invalidate_inode_pages2(blkif->vbd.bdev->bd_inode->i_mapping);
- blkif->xenblkd = kthread_run(xen_blkif_schedule, blkif, name);
+ blkif->xenblkd = kthread_run(xen_blkif_schedule, blkif, "%s", name);
if (IS_ERR(blkif->xenblkd)) {
err = PTR_ERR(blkif->xenblkd);
blkif->xenblkd = NULL;
diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
index b83bf4b..0f34bca 100644
--- a/drivers/hwmon/adt7470.c
+++ b/drivers/hwmon/adt7470.c
@@ -1285,7 +1285,7 @@ static int adt7470_probe(struct i2c_client *client,
}
init_completion(&data->auto_update_stop);
- data->auto_update = kthread_run(adt7470_update_thread, client,
+ data->auto_update = kthread_run(adt7470_update_thread, client, "%s",
dev_name(data->hwmon_dev));
if (IS_ERR(data->auto_update)) {
err = PTR_ERR(data->auto_update);
diff --git a/drivers/media/i2c/tvaudio.c b/drivers/media/i2c/tvaudio.c
index b72a59d..e0634c8 100644
--- a/drivers/media/i2c/tvaudio.c
+++ b/drivers/media/i2c/tvaudio.c
@@ -2020,7 +2020,8 @@ static int tvaudio_probe(struct i2c_client *client, const struct i2c_device_id *
/* start async thread */
chip->wt.function = chip_thread_wake;
chip->wt.data = (unsigned long)chip;
- chip->thread = kthread_run(chip_thread, chip, client->name);
+ chip->thread = kthread_run(chip_thread, chip, "%s",
+ client->name);
if (IS_ERR(chip->thread)) {
v4l2_warn(sd, "failed to create kthread\n");
chip->thread = NULL;
diff --git a/drivers/media/pci/ivtv/ivtv-driver.c b/drivers/media/pci/ivtv/ivtv-driver.c
index 07b8460..b809bc8 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.c
+++ b/drivers/media/pci/ivtv/ivtv-driver.c
@@ -753,7 +753,7 @@ static int ivtv_init_struct1(struct ivtv *itv)
init_kthread_worker(&itv->irq_worker);
itv->irq_worker_task = kthread_run(kthread_worker_fn, &itv->irq_worker,
- itv->v4l2_dev.name);
+ "%s", itv->v4l2_dev.name);
if (IS_ERR(itv->irq_worker_task)) {
IVTV_ERR("Could not create ivtv task\n");
return -1;
diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 85bc314..1d3f119 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -768,7 +768,8 @@ static int vivi_start_generating(struct vivi_dev *dev)
dma_q->frame = 0;
dma_q->ini_jiffies = jiffies;
- dma_q->kthread = kthread_run(vivi_thread, dev, dev->v4l2_dev.name);
+ dma_q->kthread = kthread_run(vivi_thread, dev, "%s",
+ dev->v4l2_dev.name);
if (IS_ERR(dma_q->kthread)) {
v4l2_err(&dev->v4l2_dev, "kernel_thread() failed\n");
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index a561335..0aaece9 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -1005,7 +1005,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
if (err)
goto out_uif;
- ubi->bgt_thread = kthread_create(ubi_thread, ubi, ubi->bgt_name);
+ ubi->bgt_thread = kthread_create(ubi_thread, ubi, "%s", ubi->bgt_name);
if (IS_ERR(ubi->bgt_thread)) {
err = PTR_ERR(ubi->bgt_thread);
ubi_err("cannot spawn \"%s\", error %d", ubi->bgt_name,
diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index 6125adb..d0adbaf 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -1893,7 +1893,8 @@ static int airo_open(struct net_device *dev) {
if (ai->wifidev != dev) {
clear_bit(JOB_DIE, &ai->jobs);
- ai->airo_thread_task = kthread_run(airo_thread, dev, dev->name);
+ ai->airo_thread_task = kthread_run(airo_thread, dev, "%s",
+ dev->name);
if (IS_ERR(ai->airo_thread_task))
return (int)PTR_ERR(ai->airo_thread_task);
diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c
index 1ef041b..d85ac1a 100644
--- a/drivers/scsi/aacraid/commctrl.c
+++ b/drivers/scsi/aacraid/commctrl.c
@@ -318,7 +318,8 @@ return_fib:
kthread_stop(dev->thread);
ssleep(1);
dev->aif_thread = 0;
- dev->thread = kthread_run(aac_command_thread, dev, dev->name);
+ dev->thread = kthread_run(aac_command_thread, dev,
+ "%s", dev->name);
ssleep(1);
}
if (f.wait) {
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c
index 1be0776..cab190a 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -1336,7 +1336,8 @@ static int _aac_reset_adapter(struct aac_dev *aac, int forced)
if ((retval = pci_set_dma_mask(aac->pdev, DMA_BIT_MASK(32))))
goto out;
if (jafo) {
- aac->thread = kthread_run(aac_command_thread, aac, aac->name);
+ aac->thread = kthread_run(aac_command_thread, aac, "%s",
+ aac->name);
if (IS_ERR(aac->thread)) {
retval = PTR_ERR(aac->thread);
goto out;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 32b7bb1..085db8b 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -601,7 +601,7 @@ static int spi_init_queue(struct spi_master *master)
init_kthread_worker(&master->kworker);
master->kworker_task = kthread_run(kthread_worker_fn,
- &master->kworker,
+ &master->kworker, "%s",
dev_name(&master->dev));
if (IS_ERR(master->kworker_task)) {
dev_err(&master->dev, "failed to create message pump task\n");
diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index b65bf5e..6e81ba0 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -238,7 +238,7 @@ struct net_device *r8712_init_netdev(void)
static u32 start_drv_threads(struct _adapter *padapter)
{
- padapter->cmdThread = kthread_run(r8712_cmd_thread, padapter,
+ padapter->cmdThread = kthread_run(r8712_cmd_thread, padapter, "%s",
padapter->pnetdev->name);
if (IS_ERR(padapter->cmdThread) < 0)
return _FAIL;
diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
index d3527dd..5e0d33a 100644
--- a/drivers/usb/atm/usbatm.c
+++ b/drivers/usb/atm/usbatm.c
@@ -1020,7 +1020,7 @@ static int usbatm_heavy_init(struct usbatm_data *instance)
{
struct task_struct *t;
- t = kthread_create(usbatm_do_heavy_init, instance,
+ t = kthread_create(usbatm_do_heavy_init, instance, "%s",
instance->driver->driver_name);
if (IS_ERR(t)) {
usb_err(instance, "%s: failed to create kernel_thread (%ld)!\n",
@@ -1076,7 +1076,8 @@ int usbatm_usb_probe(struct usb_interface *intf, const struct usb_device_id *id,
/* public fields */
instance->driver = driver;
- snprintf(instance->driver_name, sizeof(instance->driver_name), driver->driver_name);
+ strlcpy(instance->driver_name, driver->driver_name,
+ sizeof(instance->driver_name));
instance->usb_dev = usb_dev;
instance->usb_intf = intf;
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index a2aa97d..10d6c41 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -305,7 +305,7 @@ static int lockd_start_svc(struct svc_serv *serv)
svc_sock_update_bufs(serv);
serv->sv_maxconn = nlm_max_connections;
- nlmsvc_task = kthread_run(lockd, nlmsvc_rqst, serv->sv_name);
+ nlmsvc_task = kthread_run(lockd, nlmsvc_rqst, "%s", serv->sv_name);
if (IS_ERR(nlmsvc_task)) {
error = PTR_ERR(nlmsvc_task);
printk(KERN_WARNING
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index cff089a..da6a43d 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -211,7 +211,6 @@ static int nfs_callback_start_svc(int minorversion, struct rpc_xprt *xprt,
struct svc_rqst *rqstp;
int (*callback_svc)(void *vrqstp);
struct nfs_callback_data *cb_info = &nfs_callback_info[minorversion];
- char svc_name[12];
int ret;
nfs_callback_bc_serv(minorversion, xprt, serv);
@@ -235,10 +234,10 @@ static int nfs_callback_start_svc(int minorversion, struct rpc_xprt *xprt,
svc_sock_update_bufs(serv);
- sprintf(svc_name, "nfsv4.%u-svc", minorversion);
cb_info->serv = serv;
cb_info->rqst = rqstp;
- cb_info->task = kthread_run(callback_svc, cb_info->rqst, svc_name);
+ cb_info->task = kthread_run(callback_svc, cb_info->rqst,
+ "nfsv4.%u-svc", minorversion);
if (IS_ERR(cb_info->task)) {
ret = PTR_ERR(cb_info->task);
svc_exit_thread(cb_info->rqst);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 1fab140..73d0f75 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1194,7 +1194,7 @@ void nfs4_schedule_state_manager(struct nfs_client *clp)
snprintf(buf, sizeof(buf), "%s-manager",
rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR));
rcu_read_unlock();
- task = kthread_run(nfs4_run_state_manager, clp, buf);
+ task = kthread_run(nfs4_run_state_manager, clp, "%s", buf);
if (IS_ERR(task)) {
printk(KERN_ERR "%s: kthread_run: %ld\n",
__func__, PTR_ERR(task));
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 16ea679..ea7fe31 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -3108,7 +3108,7 @@ static int __init rcu_spawn_gp_kthread(void)
struct task_struct *t;
for_each_rcu_flavor(rsp) {
- t = kthread_run(rcu_gp_kthread, rsp, rsp->name);
+ t = kthread_run(rcu_gp_kthread, rsp, "%s", rsp->name);
BUG_ON(IS_ERR(t));
rnp = rcu_get_root(rsp);
raw_spin_lock_irqsave(&rnp->lock, flags);
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 89a588b..b974571 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -740,7 +740,7 @@ svc_set_num_threads(struct svc_serv *serv, struct svc_pool *pool, int nrservs)
__module_get(serv->sv_module);
task = kthread_create_on_node(serv->sv_function, rqstp,
- node, serv->sv_name);
+ node, "%s", serv->sv_name);
if (IS_ERR(task)) {
error = PTR_ERR(task);
module_put(serv->sv_module);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/8] isdn: clean up debug format string usage
2013-06-06 20:52 [PATCH 0/8] format string usage clean ups Kees Cook
` (6 preceding siblings ...)
2013-06-06 20:52 ` [PATCH 7/8] kthread: avoid parsing names as format strings Kees Cook
@ 2013-06-06 20:52 ` Kees Cook
2013-06-07 18:32 ` Kees Cook
7 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2013-06-06 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook
Avoid unneeded local string buffers for constructing debug output. Also
cleans up debug calls that contain a single parameter so that they cannot
be accidentally parsed as format strings.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Karsten Keil <isdn@linux-pingi.de>
---
drivers/isdn/hisax/amd7930_fn.c | 4 +--
drivers/isdn/hisax/avm_pci.c | 4 +--
drivers/isdn/hisax/config.c | 2 +-
drivers/isdn/hisax/diva.c | 4 +--
drivers/isdn/hisax/elsa.c | 2 +-
drivers/isdn/hisax/elsa_ser.c | 2 +-
drivers/isdn/hisax/hfc_pci.c | 2 +-
drivers/isdn/hisax/hfc_sx.c | 2 +-
drivers/isdn/hisax/hscx_irq.c | 4 +--
drivers/isdn/hisax/icc.c | 4 +--
drivers/isdn/hisax/ipacx.c | 8 +++---
drivers/isdn/hisax/isac.c | 4 +--
drivers/isdn/hisax/isar.c | 6 ++---
drivers/isdn/hisax/jade.c | 18 ++++---------
drivers/isdn/hisax/jade_irq.c | 4 +--
drivers/isdn/hisax/l3_1tr6.c | 50 +++++++++++++------------------------
drivers/isdn/hisax/netjet.c | 2 +-
drivers/isdn/hisax/q931.c | 6 ++---
drivers/isdn/hisax/w6692.c | 8 +++---
drivers/isdn/mISDN/dsp_pipeline.c | 2 +-
20 files changed, 58 insertions(+), 80 deletions(-)
diff --git a/drivers/isdn/hisax/amd7930_fn.c b/drivers/isdn/hisax/amd7930_fn.c
index 1063bab..36817e0 100644
--- a/drivers/isdn/hisax/amd7930_fn.c
+++ b/drivers/isdn/hisax/amd7930_fn.c
@@ -314,7 +314,7 @@ Amd7930_empty_Dfifo(struct IsdnCardState *cs, int flag)
t += sprintf(t, "Amd7930: empty_Dfifo cnt: %d |", cs->rcvidx);
QuickHex(t, cs->rcvbuf, cs->rcvidx);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
/* moves received data in sk-buffer */
memcpy(skb_put(skb, cs->rcvidx), cs->rcvbuf, cs->rcvidx);
@@ -406,7 +406,7 @@ Amd7930_fill_Dfifo(struct IsdnCardState *cs)
t += sprintf(t, "Amd7930: fill_Dfifo cnt: %d |", count);
QuickHex(t, deb_ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
/* AMD interrupts on */
AmdIrqOn(cs);
diff --git a/drivers/isdn/hisax/avm_pci.c b/drivers/isdn/hisax/avm_pci.c
index ee9b9a0..d1427bd 100644
--- a/drivers/isdn/hisax/avm_pci.c
+++ b/drivers/isdn/hisax/avm_pci.c
@@ -285,7 +285,7 @@ hdlc_empty_fifo(struct BCState *bcs, int count)
t += sprintf(t, "hdlc_empty_fifo %c cnt %d",
bcs->channel ? 'B' : 'A', count);
QuickHex(t, p, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
@@ -345,7 +345,7 @@ hdlc_fill_fifo(struct BCState *bcs)
t += sprintf(t, "hdlc_fill_fifo %c cnt %d",
bcs->channel ? 'B' : 'A', count);
QuickHex(t, p, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
diff --git a/drivers/isdn/hisax/config.c b/drivers/isdn/hisax/config.c
index bf04d2a..b33f53b 100644
--- a/drivers/isdn/hisax/config.c
+++ b/drivers/isdn/hisax/config.c
@@ -1896,7 +1896,7 @@ static void EChannel_proc_rcv(struct hisax_d_if *d_if)
ptr--;
*ptr++ = '\n';
*ptr = 0;
- HiSax_putstatus(cs, NULL, cs->dlog);
+ HiSax_putstatus(cs, NULL, "%s", cs->dlog);
} else
HiSax_putstatus(cs, "LogEcho: ",
"warning Frame too big (%d)",
diff --git a/drivers/isdn/hisax/diva.c b/drivers/isdn/hisax/diva.c
index 8d0cf6e..4fc90de6 100644
--- a/drivers/isdn/hisax/diva.c
+++ b/drivers/isdn/hisax/diva.c
@@ -427,7 +427,7 @@ Memhscx_empty_fifo(struct BCState *bcs, int count)
t += sprintf(t, "hscx_empty_fifo %c cnt %d",
bcs->hw.hscx.hscx ? 'B' : 'A', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
@@ -469,7 +469,7 @@ Memhscx_fill_fifo(struct BCState *bcs)
t += sprintf(t, "hscx_fill_fifo %c cnt %d",
bcs->hw.hscx.hscx ? 'B' : 'A', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
diff --git a/drivers/isdn/hisax/elsa.c b/drivers/isdn/hisax/elsa.c
index 1df6f9a..2be1c8a 100644
--- a/drivers/isdn/hisax/elsa.c
+++ b/drivers/isdn/hisax/elsa.c
@@ -535,7 +535,7 @@ check_arcofi(struct IsdnCardState *cs)
t = tmp;
t += sprintf(tmp, "Arcofi data");
QuickHex(t, p, cs->dc.isac.mon_rxp);
- debugl1(cs, tmp);
+ debugl1(cs, "%s", tmp);
if ((cs->dc.isac.mon_rxp == 2) && (cs->dc.isac.mon_rx[0] == 0xa0)) {
switch (cs->dc.isac.mon_rx[1]) {
case 0x80:
diff --git a/drivers/isdn/hisax/elsa_ser.c b/drivers/isdn/hisax/elsa_ser.c
index d4c98d3..3f84dd8 100644
--- a/drivers/isdn/hisax/elsa_ser.c
+++ b/drivers/isdn/hisax/elsa_ser.c
@@ -344,7 +344,7 @@ static inline void receive_chars(struct IsdnCardState *cs,
t += sprintf(t, "modem read cnt %d", cs->hw.elsa.rcvcnt);
QuickHex(t, cs->hw.elsa.rcvbuf, cs->hw.elsa.rcvcnt);
- debugl1(cs, tmp);
+ debugl1(cs, "%s", tmp);
}
cs->hw.elsa.rcvcnt = 0;
}
diff --git a/drivers/isdn/hisax/hfc_pci.c b/drivers/isdn/hisax/hfc_pci.c
index 3ccd724..497bd02 100644
--- a/drivers/isdn/hisax/hfc_pci.c
+++ b/drivers/isdn/hisax/hfc_pci.c
@@ -901,7 +901,7 @@ Begin:
ptr--;
*ptr++ = '\n';
*ptr = 0;
- HiSax_putstatus(cs, NULL, cs->dlog);
+ HiSax_putstatus(cs, NULL, "%s", cs->dlog);
} else
HiSax_putstatus(cs, "LogEcho: ", "warning Frame too big (%d)", total - 3);
}
diff --git a/drivers/isdn/hisax/hfc_sx.c b/drivers/isdn/hisax/hfc_sx.c
index dc4574f..fa1fefd 100644
--- a/drivers/isdn/hisax/hfc_sx.c
+++ b/drivers/isdn/hisax/hfc_sx.c
@@ -674,7 +674,7 @@ receive_emsg(struct IsdnCardState *cs)
ptr--;
*ptr++ = '\n';
*ptr = 0;
- HiSax_putstatus(cs, NULL, cs->dlog);
+ HiSax_putstatus(cs, NULL, "%s", cs->dlog);
} else
HiSax_putstatus(cs, "LogEcho: ", "warning Frame too big (%d)", skb->len);
}
diff --git a/drivers/isdn/hisax/hscx_irq.c b/drivers/isdn/hisax/hscx_irq.c
index f398d48..a8d6188 100644
--- a/drivers/isdn/hisax/hscx_irq.c
+++ b/drivers/isdn/hisax/hscx_irq.c
@@ -75,7 +75,7 @@ hscx_empty_fifo(struct BCState *bcs, int count)
t += sprintf(t, "hscx_empty_fifo %c cnt %d",
bcs->hw.hscx.hscx ? 'B' : 'A', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
@@ -115,7 +115,7 @@ hscx_fill_fifo(struct BCState *bcs)
t += sprintf(t, "hscx_fill_fifo %c cnt %d",
bcs->hw.hscx.hscx ? 'B' : 'A', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
diff --git a/drivers/isdn/hisax/icc.c b/drivers/isdn/hisax/icc.c
index db5321f..51dae91 100644
--- a/drivers/isdn/hisax/icc.c
+++ b/drivers/isdn/hisax/icc.c
@@ -134,7 +134,7 @@ icc_empty_fifo(struct IsdnCardState *cs, int count)
t += sprintf(t, "icc_empty_fifo cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
@@ -176,7 +176,7 @@ icc_fill_fifo(struct IsdnCardState *cs)
t += sprintf(t, "icc_fill_fifo cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
diff --git a/drivers/isdn/hisax/ipacx.c b/drivers/isdn/hisax/ipacx.c
index 74feb5c..5faa5de 100644
--- a/drivers/isdn/hisax/ipacx.c
+++ b/drivers/isdn/hisax/ipacx.c
@@ -260,7 +260,7 @@ dch_empty_fifo(struct IsdnCardState *cs, int count)
t += sprintf(t, "dch_empty_fifo() cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
@@ -307,7 +307,7 @@ dch_fill_fifo(struct IsdnCardState *cs)
t += sprintf(t, "dch_fill_fifo() cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
@@ -539,7 +539,7 @@ bch_empty_fifo(struct BCState *bcs, int count)
t += sprintf(t, "bch_empty_fifo() B-%d cnt %d", hscx, count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
@@ -582,7 +582,7 @@ bch_fill_fifo(struct BCState *bcs)
t += sprintf(t, "chb_fill_fifo() B-%d cnt %d", hscx, count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
diff --git a/drivers/isdn/hisax/isac.c b/drivers/isdn/hisax/isac.c
index a365ccc..7fdf78f 100644
--- a/drivers/isdn/hisax/isac.c
+++ b/drivers/isdn/hisax/isac.c
@@ -137,7 +137,7 @@ isac_empty_fifo(struct IsdnCardState *cs, int count)
t += sprintf(t, "isac_empty_fifo cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
@@ -179,7 +179,7 @@ isac_fill_fifo(struct IsdnCardState *cs)
t += sprintf(t, "isac_fill_fifo cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
diff --git a/drivers/isdn/hisax/isar.c b/drivers/isdn/hisax/isar.c
index 7fdf347..f4956c7 100644
--- a/drivers/isdn/hisax/isar.c
+++ b/drivers/isdn/hisax/isar.c
@@ -74,7 +74,7 @@ sendmsg(struct IsdnCardState *cs, u_char his, u_char creg, u_char len,
t = tmp;
t += sprintf(t, "sendmbox cnt %d", len);
QuickHex(t, &msg[len-i], (i > 64) ? 64 : i);
- debugl1(cs, tmp);
+ debugl1(cs, "%s", tmp);
i -= 64;
}
}
@@ -105,7 +105,7 @@ rcv_mbox(struct IsdnCardState *cs, struct isar_reg *ireg, u_char *msg)
t = tmp;
t += sprintf(t, "rcv_mbox cnt %d", ireg->clsb);
QuickHex(t, &msg[ireg->clsb - i], (i > 64) ? 64 : i);
- debugl1(cs, tmp);
+ debugl1(cs, "%s", tmp);
i -= 64;
}
}
@@ -1248,7 +1248,7 @@ isar_int_main(struct IsdnCardState *cs)
tp += sprintf(debbuf, "msg iis(%x) msb(%x)",
ireg->iis, ireg->cmsb);
QuickHex(tp, (u_char *)ireg->par, ireg->clsb);
- debugl1(cs, debbuf);
+ debugl1(cs, "%s", debbuf);
}
break;
case ISAR_IIS_INVMSG:
diff --git a/drivers/isdn/hisax/jade.c b/drivers/isdn/hisax/jade.c
index f946c58..e2ae787 100644
--- a/drivers/isdn/hisax/jade.c
+++ b/drivers/isdn/hisax/jade.c
@@ -81,10 +81,7 @@ modejade(struct BCState *bcs, int mode, int bc)
int jade = bcs->hw.hscx.hscx;
if (cs->debug & L1_DEB_HSCX) {
- char tmp[40];
- sprintf(tmp, "jade %c mode %d ichan %d",
- 'A' + jade, mode, bc);
- debugl1(cs, tmp);
+ debugl1(cs, "jade %c mode %d ichan %d", 'A' + jade, mode, bc);
}
bcs->mode = mode;
bcs->channel = bc;
@@ -257,23 +254,18 @@ void
clear_pending_jade_ints(struct IsdnCardState *cs)
{
int val;
- char tmp[64];
cs->BC_Write_Reg(cs, 0, jade_HDLC_IMR, 0x00);
cs->BC_Write_Reg(cs, 1, jade_HDLC_IMR, 0x00);
val = cs->BC_Read_Reg(cs, 1, jade_HDLC_ISR);
- sprintf(tmp, "jade B ISTA %x", val);
- debugl1(cs, tmp);
+ debugl1(cs, "jade B ISTA %x", val);
val = cs->BC_Read_Reg(cs, 0, jade_HDLC_ISR);
- sprintf(tmp, "jade A ISTA %x", val);
- debugl1(cs, tmp);
+ debugl1(cs, "jade A ISTA %x", val);
val = cs->BC_Read_Reg(cs, 1, jade_HDLC_STAR);
- sprintf(tmp, "jade B STAR %x", val);
- debugl1(cs, tmp);
+ debugl1(cs, "jade B STAR %x", val);
val = cs->BC_Read_Reg(cs, 0, jade_HDLC_STAR);
- sprintf(tmp, "jade A STAR %x", val);
- debugl1(cs, tmp);
+ debugl1(cs, "jade A STAR %x", val);
/* Unmask ints */
cs->BC_Write_Reg(cs, 0, jade_HDLC_IMR, 0xF8);
cs->BC_Write_Reg(cs, 1, jade_HDLC_IMR, 0xF8);
diff --git a/drivers/isdn/hisax/jade_irq.c b/drivers/isdn/hisax/jade_irq.c
index f521fc8..b930da9 100644
--- a/drivers/isdn/hisax/jade_irq.c
+++ b/drivers/isdn/hisax/jade_irq.c
@@ -65,7 +65,7 @@ jade_empty_fifo(struct BCState *bcs, int count)
t += sprintf(t, "jade_empty_fifo %c cnt %d",
bcs->hw.hscx.hscx ? 'B' : 'A', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
@@ -105,7 +105,7 @@ jade_fill_fifo(struct BCState *bcs)
t += sprintf(t, "jade_fill_fifo %c cnt %d",
bcs->hw.hscx.hscx ? 'B' : 'A', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
diff --git a/drivers/isdn/hisax/l3_1tr6.c b/drivers/isdn/hisax/l3_1tr6.c
index 4c1bca5..875402e 100644
--- a/drivers/isdn/hisax/l3_1tr6.c
+++ b/drivers/isdn/hisax/l3_1tr6.c
@@ -63,7 +63,7 @@ l3_1tr6_error(struct l3_process *pc, u_char *msg, struct sk_buff *skb)
{
dev_kfree_skb(skb);
if (pc->st->l3.debug & L3_DEB_WARN)
- l3_debug(pc->st, msg);
+ l3_debug(pc->st, "%s", msg);
l3_1tr6_release_req(pc, 0, NULL);
}
@@ -161,7 +161,6 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
{
u_char *p;
int bcfound = 0;
- char tmp[80];
struct sk_buff *skb = arg;
/* Channel Identification */
@@ -214,10 +213,9 @@ l3_1tr6_setup(struct l3_process *pc, u_char pr, void *arg)
/* Signal all services, linklevel takes care of Service-Indicator */
if (bcfound) {
if ((pc->para.setup.si1 != 7) && (pc->st->l3.debug & L3_DEB_WARN)) {
- sprintf(tmp, "non-digital call: %s -> %s",
+ l3_debug(pc->st, "non-digital call: %s -> %s",
pc->para.setup.phone,
pc->para.setup.eazmsn);
- l3_debug(pc->st, tmp);
}
newl3state(pc, 6);
pc->st->l3.l3l4(pc->st, CC_SETUP | INDICATION, pc);
@@ -301,7 +299,7 @@ l3_1tr6_info(struct l3_process *pc, u_char pr, void *arg)
{
u_char *p;
int i, tmpcharge = 0;
- char a_charge[8], tmp[32];
+ char a_charge[8];
struct sk_buff *skb = arg;
p = skb->data;
@@ -316,8 +314,8 @@ l3_1tr6_info(struct l3_process *pc, u_char pr, void *arg)
pc->st->l3.l3l4(pc->st, CC_CHARGE | INDICATION, pc);
}
if (pc->st->l3.debug & L3_DEB_CHARGE) {
- sprintf(tmp, "charging info %d", pc->para.chargeinfo);
- l3_debug(pc->st, tmp);
+ l3_debug(pc->st, "charging info %d",
+ pc->para.chargeinfo);
}
} else if (pc->st->l3.debug & L3_DEB_CHARGE)
l3_debug(pc->st, "charging info not found");
@@ -399,7 +397,7 @@ l3_1tr6_disc(struct l3_process *pc, u_char pr, void *arg)
struct sk_buff *skb = arg;
u_char *p;
int i, tmpcharge = 0;
- char a_charge[8], tmp[32];
+ char a_charge[8];
StopAllL3Timer(pc);
p = skb->data;
@@ -414,8 +412,8 @@ l3_1tr6_disc(struct l3_process *pc, u_char pr, void *arg)
pc->st->l3.l3l4(pc->st, CC_CHARGE | INDICATION, pc);
}
if (pc->st->l3.debug & L3_DEB_CHARGE) {
- sprintf(tmp, "charging info %d", pc->para.chargeinfo);
- l3_debug(pc->st, tmp);
+ l3_debug(pc->st, "charging info %d",
+ pc->para.chargeinfo);
}
} else if (pc->st->l3.debug & L3_DEB_CHARGE)
l3_debug(pc->st, "charging info not found");
@@ -746,7 +744,6 @@ up1tr6(struct PStack *st, int pr, void *arg)
int i, mt, cr;
struct l3_process *proc;
struct sk_buff *skb = arg;
- char tmp[80];
switch (pr) {
case (DL_DATA | INDICATION):
@@ -762,26 +759,23 @@ up1tr6(struct PStack *st, int pr, void *arg)
}
if (skb->len < 4) {
if (st->l3.debug & L3_DEB_PROTERR) {
- sprintf(tmp, "up1tr6 len only %d", skb->len);
- l3_debug(st, tmp);
+ l3_debug(st, "up1tr6 len only %d", skb->len);
}
dev_kfree_skb(skb);
return;
}
if ((skb->data[0] & 0xfe) != PROTO_DIS_N0) {
if (st->l3.debug & L3_DEB_PROTERR) {
- sprintf(tmp, "up1tr6%sunexpected discriminator %x message len %d",
+ l3_debug(st, "up1tr6%sunexpected discriminator %x message len %d",
(pr == (DL_DATA | INDICATION)) ? " " : "(broadcast) ",
skb->data[0], skb->len);
- l3_debug(st, tmp);
}
dev_kfree_skb(skb);
return;
}
if (skb->data[1] != 1) {
if (st->l3.debug & L3_DEB_PROTERR) {
- sprintf(tmp, "up1tr6 CR len not 1");
- l3_debug(st, tmp);
+ l3_debug(st, "up1tr6 CR len not 1");
}
dev_kfree_skb(skb);
return;
@@ -791,9 +785,8 @@ up1tr6(struct PStack *st, int pr, void *arg)
if (skb->data[0] == PROTO_DIS_N0) {
dev_kfree_skb(skb);
if (st->l3.debug & L3_DEB_STATE) {
- sprintf(tmp, "up1tr6%s N0 mt %x unhandled",
+ l3_debug(st, "up1tr6%s N0 mt %x unhandled",
(pr == (DL_DATA | INDICATION)) ? " " : "(broadcast) ", mt);
- l3_debug(st, tmp);
}
} else if (skb->data[0] == PROTO_DIS_N1) {
if (!(proc = getl3proc(st, cr))) {
@@ -801,8 +794,7 @@ up1tr6(struct PStack *st, int pr, void *arg)
if (cr < 128) {
if (!(proc = new_l3_process(st, cr))) {
if (st->l3.debug & L3_DEB_PROTERR) {
- sprintf(tmp, "up1tr6 no roc mem");
- l3_debug(st, tmp);
+ l3_debug(st, "up1tr6 no roc mem");
}
dev_kfree_skb(skb);
return;
@@ -821,8 +813,7 @@ up1tr6(struct PStack *st, int pr, void *arg)
} else {
if (!(proc = new_l3_process(st, cr))) {
if (st->l3.debug & L3_DEB_PROTERR) {
- sprintf(tmp, "up1tr6 no roc mem");
- l3_debug(st, tmp);
+ l3_debug(st, "up1tr6 no roc mem");
}
dev_kfree_skb(skb);
return;
@@ -837,18 +828,16 @@ up1tr6(struct PStack *st, int pr, void *arg)
if (i == ARRAY_SIZE(datastln1)) {
dev_kfree_skb(skb);
if (st->l3.debug & L3_DEB_STATE) {
- sprintf(tmp, "up1tr6%sstate %d mt %x unhandled",
+ l3_debug(st, "up1tr6%sstate %d mt %x unhandled",
(pr == (DL_DATA | INDICATION)) ? " " : "(broadcast) ",
proc->state, mt);
- l3_debug(st, tmp);
}
return;
} else {
if (st->l3.debug & L3_DEB_STATE) {
- sprintf(tmp, "up1tr6%sstate %d mt %x",
+ l3_debug(st, "up1tr6%sstate %d mt %x",
(pr == (DL_DATA | INDICATION)) ? " " : "(broadcast) ",
proc->state, mt);
- l3_debug(st, tmp);
}
datastln1[i].rout(proc, pr, skb);
}
@@ -861,7 +850,6 @@ down1tr6(struct PStack *st, int pr, void *arg)
int i, cr;
struct l3_process *proc;
struct Channel *chan;
- char tmp[80];
if ((DL_ESTABLISH | REQUEST) == pr) {
l3_msg(st, pr, NULL);
@@ -888,15 +876,13 @@ down1tr6(struct PStack *st, int pr, void *arg)
break;
if (i == ARRAY_SIZE(downstl)) {
if (st->l3.debug & L3_DEB_STATE) {
- sprintf(tmp, "down1tr6 state %d prim %d unhandled",
+ l3_debug(st, "down1tr6 state %d prim %d unhandled",
proc->state, pr);
- l3_debug(st, tmp);
}
} else {
if (st->l3.debug & L3_DEB_STATE) {
- sprintf(tmp, "down1tr6 state %d prim %d",
+ l3_debug(st, "down1tr6 state %d prim %d",
proc->state, pr);
- l3_debug(st, tmp);
}
downstl[i].rout(proc, pr, arg);
}
diff --git a/drivers/isdn/hisax/netjet.c b/drivers/isdn/hisax/netjet.c
index b646eed..233e432 100644
--- a/drivers/isdn/hisax/netjet.c
+++ b/drivers/isdn/hisax/netjet.c
@@ -176,7 +176,7 @@ static void printframe(struct IsdnCardState *cs, u_char *buf, int count, char *s
else
j = i;
QuickHex(t, p, j);
- debugl1(cs, tmp);
+ debugl1(cs, "%s", tmp);
p += j;
i -= j;
t = tmp;
diff --git a/drivers/isdn/hisax/q931.c b/drivers/isdn/hisax/q931.c
index 041bf52..af1b020 100644
--- a/drivers/isdn/hisax/q931.c
+++ b/drivers/isdn/hisax/q931.c
@@ -1179,7 +1179,7 @@ LogFrame(struct IsdnCardState *cs, u_char *buf, int size)
dp--;
*dp++ = '\n';
*dp = 0;
- HiSax_putstatus(cs, NULL, cs->dlog);
+ HiSax_putstatus(cs, NULL, "%s", cs->dlog);
} else
HiSax_putstatus(cs, "LogFrame: ", "warning Frame too big (%d)", size);
}
@@ -1246,7 +1246,7 @@ dlogframe(struct IsdnCardState *cs, struct sk_buff *skb, int dir)
}
if (finish) {
*dp = 0;
- HiSax_putstatus(cs, NULL, cs->dlog);
+ HiSax_putstatus(cs, NULL, "%s", cs->dlog);
return;
}
if ((0xfe & buf[0]) == PROTO_DIS_N0) { /* 1TR6 */
@@ -1509,5 +1509,5 @@ dlogframe(struct IsdnCardState *cs, struct sk_buff *skb, int dir)
dp += sprintf(dp, "Unknown protocol %x!", buf[0]);
}
*dp = 0;
- HiSax_putstatus(cs, NULL, cs->dlog);
+ HiSax_putstatus(cs, NULL, "%s", cs->dlog);
}
diff --git a/drivers/isdn/hisax/w6692.c b/drivers/isdn/hisax/w6692.c
index d8cac69..a858955 100644
--- a/drivers/isdn/hisax/w6692.c
+++ b/drivers/isdn/hisax/w6692.c
@@ -154,7 +154,7 @@ W6692_empty_fifo(struct IsdnCardState *cs, int count)
t += sprintf(t, "W6692_empty_fifo cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
@@ -196,7 +196,7 @@ W6692_fill_fifo(struct IsdnCardState *cs)
t += sprintf(t, "W6692_fill_fifo cnt %d", count);
QuickHex(t, ptr, count);
- debugl1(cs, cs->dlog);
+ debugl1(cs, "%s", cs->dlog);
}
}
@@ -226,7 +226,7 @@ W6692B_empty_fifo(struct BCState *bcs, int count)
t += sprintf(t, "W6692B_empty_fifo %c cnt %d",
bcs->channel + '1', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
@@ -264,7 +264,7 @@ W6692B_fill_fifo(struct BCState *bcs)
t += sprintf(t, "W6692B_fill_fifo %c cnt %d",
bcs->channel + '1', count);
QuickHex(t, ptr, count);
- debugl1(cs, bcs->blog);
+ debugl1(cs, "%s", bcs->blog);
}
}
diff --git a/drivers/isdn/mISDN/dsp_pipeline.c b/drivers/isdn/mISDN/dsp_pipeline.c
index 8b1a66c..88305c9 100644
--- a/drivers/isdn/mISDN/dsp_pipeline.c
+++ b/drivers/isdn/mISDN/dsp_pipeline.c
@@ -102,7 +102,7 @@ int mISDN_dsp_element_register(struct mISDN_dsp_element *elem)
entry->dev.class = elements_class;
entry->dev.release = mISDN_dsp_dev_release;
dev_set_drvdata(&entry->dev, elem);
- dev_set_name(&entry->dev, "%s", elem->name);
+ dev_set_name(&entry->dev, elem->name);
ret = device_register(&entry->dev);
if (ret) {
printk(KERN_ERR "%s: failed to register %s\n",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 8/8] isdn: clean up debug format string usage
2013-06-06 20:52 ` [PATCH 8/8] isdn: clean up debug format string usage Kees Cook
@ 2013-06-07 18:32 ` Kees Cook
0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2013-06-07 18:32 UTC (permalink / raw)
To: LKML
Cc: Andrew Morton, Greg Kroah-Hartman, Jens Axboe, David S. Miller,
Herbert Xu, David Woodhouse, Anton Vorontsov, Karsten Keil,
Kees Cook
On Thu, Jun 6, 2013 at 1:52 PM, Kees Cook <keescook@chromium.org> wrote:
> Avoid unneeded local string buffers for constructing debug output. Also
> cleans up debug calls that contain a single parameter so that they cannot
> be accidentally parsed as format strings.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: Karsten Keil <isdn@linux-pingi.de>
> [...]
> diff --git a/drivers/isdn/mISDN/dsp_pipeline.c b/drivers/isdn/mISDN/dsp_pipeline.c
> index 8b1a66c..88305c9 100644
> --- a/drivers/isdn/mISDN/dsp_pipeline.c
> +++ b/drivers/isdn/mISDN/dsp_pipeline.c
> @@ -102,7 +102,7 @@ int mISDN_dsp_element_register(struct mISDN_dsp_element *elem)
> entry->dev.class = elements_class;
> entry->dev.release = mISDN_dsp_dev_release;
> dev_set_drvdata(&entry->dev, elem);
> - dev_set_name(&entry->dev, "%s", elem->name);
> + dev_set_name(&entry->dev, elem->name);
> ret = device_register(&entry->dev);
> if (ret) {
> printk(KERN_ERR "%s: failed to register %s\n",
This hunk should be dropped -- it got mixed up when splitting the
dev_set_name() patch out.
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 15+ messages in thread