Re: [PATCH] fix calls to request_module()
From: Nguyen Anh Quynh
Date: Thu Dec 11 2008 - 01:29:25 EST
On Thu, Dec 11, 2008 at 3:00 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> On Thu, Dec 11, 2008 at 02:42:28PM +0900, Nguyen Anh Quynh wrote:
>> > Ah, but that's different. Take a look at that warning and think _why_
>> > it is given and what is it about. Getting an untrusted string as
>> > format argument is a real security hole, but it has nothing to do
>> > with a pile of cases in your patch.
>>
>> Yes, clearly the warning is to warn us about potential format string
>> bugs. But I agree that there are a lot of false possitives.
>>
>> My patch is mainly to make gcc happy.
>
> Your patch is mostly obfuscating the places gcc does *not* warn about...
>
> Looking through it, only
> drivers/media/video/cx18/cx18-driver.c
> drivers/media/video/ivtv/ivtv-driver.c
> drivers/media/video/pvrusb2/pvrusb2-hdw.c
> drivers/media/video/zoran/zoran_card.c
> drivers/mtd/chips/gen_probe.c
> drivers/net/wireless/hostap/hostap_ioctl.c
> drivers/of/of_spi.c
> drivers/usb/storage/libusual.c
> fs/dquot.c
> fs/gfs2/locking.c
> net/ieee80211/ieee80211_wx.c
> sound/core/sound.c
>
> are not of <string literal> -> "%s", <string literal> variety. Everything
> else has never generated a format warning. At least trim the patch,
> removing the obviously useless parts.
Yes, initially I misunderstood these warnings.
So here is the patch, again, with only the "correct" warning fixed.
Signed-off-by: Nguyen Anh Quynh <aquynh@xxxxxxxxx>
Thanks,
Quynh
# diffstat fix-request_module-call.linus_git_tree.patch
drivers/media/video/cx18/cx18-driver.c | 2 +-
drivers/media/video/ivtv/ivtv-driver.c | 2 +-
drivers/media/video/pvrusb2/pvrusb2-hdw.c | 2 +-
drivers/media/video/zoran/zoran_card.c | 8 ++++----
drivers/mtd/chips/gen_probe.c | 2 +-
drivers/net/wireless/hostap/hostap_ioctl.c | 2 +-
drivers/of/of_spi.c | 2 +-
drivers/usb/storage/libusual.c | 2 +-
fs/dquot.c | 2 +-
fs/gfs2/locking.c | 2 +-
net/ieee80211/ieee80211_wx.c | 2 +-
sound/core/sound.c | 2 +-
12 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/media/video/cx18/cx18-driver.c b/drivers/media/video/cx18/cx18-driver.c
index 7874d97..81f0d5e 100644
--- a/drivers/media/video/cx18/cx18-driver.c
+++ b/drivers/media/video/cx18/cx18-driver.c
@@ -572,7 +572,7 @@ static u32 cx18_request_module(struct cx18 *cx, u32 hw,
{
if ((hw & id) == 0)
return hw;
- if (request_module(name) != 0) {
+ if (request_module("%s", name) != 0) {
CX18_ERR("Failed to load module %s\n", name);
return hw & ~id;
}
diff --git a/drivers/media/video/ivtv/ivtv-driver.c b/drivers/media/video/ivtv/ivtv-driver.c
index b69cc1d..a26ad59 100644
--- a/drivers/media/video/ivtv/ivtv-driver.c
+++ b/drivers/media/video/ivtv/ivtv-driver.c
@@ -859,7 +859,7 @@ static u32 ivtv_request_module(struct ivtv *itv, u32 hw,
{
if ((hw & id) == 0)
return hw;
- if (request_module(name) != 0) {
+ if (request_module("%s", name) != 0) {
IVTV_ERR("Failed to load module %s\n", name);
return hw & ~id;
}
diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
index 5b81ba4..6d75d3c 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
@@ -1967,7 +1967,7 @@ static void pvr2_hdw_setup_low(struct pvr2_hdw *hdw)
if (!pvr2_hdw_dev_ok(hdw)) return;
for (idx = 0; idx < hdw->hdw_desc->client_modules.cnt; idx++) {
- request_module(hdw->hdw_desc->client_modules.lst[idx]);
+ request_module("%s", hdw->hdw_desc->client_modules.lst[idx]);
}
if (!hdw->hdw_desc->flag_no_powerup) {
diff --git a/drivers/media/video/zoran/zoran_card.c b/drivers/media/video/zoran/zoran_card.c
index fa5f2f8..586ee7d 100644
--- a/drivers/media/video/zoran/zoran_card.c
+++ b/drivers/media/video/zoran/zoran_card.c
@@ -1435,7 +1435,7 @@ find_zr36057 (void)
}
if (i2c_dec_name) {
- if ((result = request_module(i2c_dec_name)) < 0) {
+ if ((result = request_module("%s", i2c_dec_name)) < 0) {
dprintk(1,
KERN_ERR
"%s: failed to load module %s: %d\n",
@@ -1455,7 +1455,7 @@ find_zr36057 (void)
}
if (i2c_enc_name) {
- if ((result = request_module(i2c_enc_name)) < 0) {
+ if ((result = request_module("%s", i2c_enc_name)) < 0) {
dprintk(1,
KERN_ERR
"%s: failed to load module %s: %d\n",
@@ -1478,7 +1478,7 @@ find_zr36057 (void)
if (zr->card.video_codec != 0 &&
(codec_name =
codecid_to_modulename(zr->card.video_codec)) != NULL) {
- if ((result = request_module(codec_name)) < 0) {
+ if ((result = request_module("%s", codec_name)) < 0) {
dprintk(1,
KERN_ERR
"%s: failed to load modules %s: %d\n",
@@ -1488,7 +1488,7 @@ find_zr36057 (void)
if (zr->card.video_vfe != 0 &&
(vfe_name =
codecid_to_modulename(zr->card.video_vfe)) != NULL) {
- if ((result = request_module(vfe_name)) < 0) {
+ if ((result = request_module("%s", vfe_name)) < 0) {
dprintk(1,
KERN_ERR
"%s: failed to load modules %s: %d\n",
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index e2dc964..814d4f3 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -212,7 +212,7 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map,
probe_function = __symbol_get(probename);
if (!probe_function) {
- request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
+ request_module("%s", probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
probe_function = __symbol_get(probename);
}
diff --git a/drivers/net/wireless/hostap/hostap_ioctl.c b/drivers/net/wireless/hostap/hostap_ioctl.c
index 3f8b1d7..ea03992 100644
--- a/drivers/net/wireless/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/hostap/hostap_ioctl.c
@@ -3297,7 +3297,7 @@ static int prism2_ioctl_siwencodeext(struct net_device *dev,
ops = ieee80211_get_crypto_ops(alg);
if (ops == NULL) {
- request_module(module);
+ request_module("%s", module);
ops = ieee80211_get_crypto_ops(alg);
}
if (ops == NULL) {
diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
index bed0ed6..ad96437 100644
--- a/drivers/of/of_spi.c
+++ b/drivers/of/of_spi.c
@@ -82,7 +82,7 @@ void of_register_spi_devices(struct spi_master *master, struct device_node *np)
spi->dev.archdata.of_node = nc;
/* Register the new device */
- request_module(spi->modalias);
+ request_module("%s", spi->modalias);
rc = spi_add_device(spi);
if (rc) {
dev_err(&master->dev, "spi_device register error %s\n",
diff --git a/drivers/usb/storage/libusual.c b/drivers/usb/storage/libusual.c
index d617e8a..a481e02 100644
--- a/drivers/usb/storage/libusual.c
+++ b/drivers/usb/storage/libusual.c
@@ -180,7 +180,7 @@ static int usu_probe_thread(void *arg)
unsigned long flags;
mutex_lock(&usu_probe_mutex);
- rc = request_module(bias_names[type]);
+ rc = request_module("%s", bias_names[type]);
spin_lock_irqsave(&usu_lock, flags);
if (rc == 0 && (st->fls & USU_MOD_FL_PRESENT) == 0) {
/*
diff --git a/fs/dquot.c b/fs/dquot.c
index 5e95261..3984307 100644
--- a/fs/dquot.c
+++ b/fs/dquot.c
@@ -167,7 +167,7 @@ static struct quota_format_type *find_quota_format(int id)
spin_unlock(&dq_list_lock);
for (qm = 0; module_names[qm].qm_fmt_id && module_names[qm].qm_fmt_id != id; qm++);
- if (!module_names[qm].qm_fmt_id || request_module(module_names[qm].qm_mod_name))
+ if (!module_names[qm].qm_fmt_id || request_module("%s", module_names[qm].qm_mod_name))
return NULL;
spin_lock(&dq_list_lock);
diff --git a/fs/gfs2/locking.c b/fs/gfs2/locking.c
index 523243a..7a9df7f 100644
--- a/fs/gfs2/locking.c
+++ b/fs/gfs2/locking.c
@@ -177,7 +177,7 @@ retry:
if (!try && capable(CAP_SYS_MODULE)) {
try = 1;
mutex_unlock(&lmh_lock);
- request_module(proto_name);
+ request_module("%s", proto_name);
goto retry;
}
printk(KERN_INFO "GFS2: can't find protocol %s\n", proto_name);
diff --git a/net/ieee80211/ieee80211_wx.c b/net/ieee80211/ieee80211_wx.c
index 973832d..de4be03 100644
--- a/net/ieee80211/ieee80211_wx.c
+++ b/net/ieee80211/ieee80211_wx.c
@@ -608,7 +608,7 @@ int ieee80211_wx_set_encodeext(struct ieee80211_device *ieee,
ops = ieee80211_get_crypto_ops(alg);
if (ops == NULL) {
- request_module(module);
+ request_module("%s", module);
ops = ieee80211_get_crypto_ops(alg);
}
if (ops == NULL) {
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 44a69bb..c143b9c 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -88,7 +88,7 @@ static void snd_request_other(int minor)
case SNDRV_MINOR_TIMER: str = "snd-timer"; break;
default: return;
}
- request_module(str);
+ request_module("%s", str);
}
#endif /* modular kernel */