Hello,
I spotted a few unchecked memory allocation failures recently,
and went looking for more. I found and fixed seven others.
I posted them all in one change-set, here:
Thanks again to Markus Armbruster for a rigorous review.
Subject: [PATCH] Handle memory allocation failure.
http://thread.gmane.org/gmane.linux.kernel/636710
Andrew Morton suggested posting them separately, with each
subject indicating the affected subsection, per #14 in
Documentation/SubmittingPatches, so this series is the result:
Subject: [PATCH] alpha: handle kcalloc failure
Subject: [PATCH] 9p: handle kstrdup and match_strdup failure
Subject: [PATCH] affs: handle match_strdup failure
Subject: [PATCH] hfs: handle match_strdup failure
Subject: [PATCH] hfsplus: handle match_strdup failure
Subject: [PATCH] usbaudio: handle kcalloc failure
* arch/alpha/kernel/module.c (module_frob_arch_sections): Handle kcalloc failure.
Signed-off-by: Jim Meyering <[email protected]>
---
arch/alpha/kernel/module.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/arch/alpha/kernel/module.c b/arch/alpha/kernel/module.c
index 026ba9a..ebc3c89 100644
--- a/arch/alpha/kernel/module.c
+++ b/arch/alpha/kernel/module.c
@@ -120,6 +120,12 @@ module_frob_arch_sections(Elf64_Ehdr *hdr, Elf64_Shdr *sechdrs,
nsyms = symtab->sh_size / sizeof(Elf64_Sym);
chains = kcalloc(nsyms, sizeof(struct got_entry), GFP_KERNEL);
+ if (!chains) {
+ printk(KERN_ERR
+ "module %s: no memory for symbol chain buffer\n",
+ me->name);
+ return -ENOMEM;
+ }
got->sh_size = 0;
got->sh_addralign = 8;
--
1.5.4.2.134.g82883
* fs/affs/super.c (parse_options): Remove useless initialization.
Handle match_strdup failure.
Signed-off-by: Jim Meyering <[email protected]>
---
fs/affs/super.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/affs/super.c b/fs/affs/super.c
index d2dc047..01d25d5 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -199,7 +199,6 @@ parse_options(char *options, uid_t *uid, gid_t *gid, int *mode, int *reserved, s
case Opt_prefix:
/* Free any previous prefix */
kfree(*prefix);
- *prefix = NULL;
*prefix = match_strdup(&args[0]);
if (!*prefix)
return 0;
@@ -233,6 +232,8 @@ parse_options(char *options, uid_t *uid, gid_t *gid, int *mode, int *reserved, s
break;
case Opt_volume: {
char *vol = match_strdup(&args[0]);
+ if (!vol)
+ return 0;
strlcpy(volume, vol, 32);
kfree(vol);
break;
--
1.5.4.2.134.g82883
* fs/hfs/super.c (parse_options): Handle match_strdup failure, twice.
Signed-off-by: Jim Meyering <[email protected]>
---
fs/hfs/super.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 32de44e..8cf6797 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -297,7 +297,8 @@ static int parse_options(char *options, struct hfs_sb_info *hsb)
return 0;
}
p = match_strdup(&args[0]);
- hsb->nls_disk = load_nls(p);
+ if (p)
+ hsb->nls_disk = load_nls(p);
if (!hsb->nls_disk) {
printk(KERN_ERR "hfs: unable to load codepage \"%s\"\n", p);
kfree(p);
@@ -311,7 +312,8 @@ static int parse_options(char *options, struct hfs_sb_info *hsb)
return 0;
}
p = match_strdup(&args[0]);
- hsb->nls_io = load_nls(p);
+ if (p)
+ hsb->nls_io = load_nls(p);
if (!hsb->nls_io) {
printk(KERN_ERR "hfs: unable to load iocharset \"%s\"\n", p);
kfree(p);
--
1.5.4.2.134.g82883
* fs/hfsplus/options.c (hfsplus_parse_options): Handle match_strdup failure.
Signed-off-by: Jim Meyering <[email protected]>
---
fs/hfsplus/options.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c
index dc64fac..9997cbf 100644
--- a/fs/hfsplus/options.c
+++ b/fs/hfsplus/options.c
@@ -132,7 +132,8 @@ int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi)
return 0;
}
p = match_strdup(&args[0]);
- sbi->nls = load_nls(p);
+ if (p)
+ sbi->nls = load_nls(p);
if (!sbi->nls) {
printk(KERN_ERR "hfs: unable to load nls mapping \"%s\"\n", p);
kfree(p);
--
1.5.4.2.134.g82883
* sound/usb/usbaudio.c (check_hw_params_convention): Handle kcalloc failure.
Signed-off-by: Jim Meyering <[email protected]>
---
sound/usb/usbaudio.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/sound/usb/usbaudio.c b/sound/usb/usbaudio.c
index 8fa9356..b61533d 100644
--- a/sound/usb/usbaudio.c
+++ b/sound/usb/usbaudio.c
@@ -1735,6 +1735,8 @@ static int check_hw_params_convention(struct snd_usb_substream *subs)
channels = kcalloc(MAX_MASK, sizeof(u32), GFP_KERNEL);
rates = kcalloc(MAX_MASK, sizeof(u32), GFP_KERNEL);
+ if (!channels || !rates)
+ goto __out;
list_for_each(p, &subs->fmt_list) {
struct audioformat *f;
--
1.5.4.2.134.g82883
* fs/9p/v9fs.c (v9fs_parse_options): Handle kstrdup and match_strdup failure.
Now that this function can fail, return an int, diagnose other option-parsing
failures, and adjust the sole caller:
(v9fs_session_init): Handle kstrdup failure. Propagate any new
v9fs_parse_options failure "up".
Signed-off-by: Jim Meyering <[email protected]>
---
FYI, my first crack at cleaning up v9fs_parse_options
removed the match_strdup call altogether, but was more
invasive in that it added a new function, match_strcmp and
made the existing match_number function public.
The patch below just handles the match_strdup failure.
fs/9p/v9fs.c | 40 ++++++++++++++++++++++++++++++++++------
1 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 9b0f022..8ca142c 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -74,16 +74,17 @@ static match_table_t tokens = {
* @options: options string passed from mount
* @v9ses: existing v9fs session information
*
+ * Return 0 upon success, -ERRNO upon failure.
*/
-static void v9fs_parse_options(struct v9fs_session_info *v9ses)
+static int v9fs_parse_options(struct v9fs_session_info *v9ses)
{
char *options;
substring_t args[MAX_OPT_ARGS];
char *p;
int option = 0;
char *s, *e;
- int ret;
+ int ret = 0;
/* setup defaults */
v9ses->afid = ~0;
@@ -91,19 +92,26 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses)
v9ses->cache = 0;
if (!v9ses->options)
- return;
+ return 0;
options = kstrdup(v9ses->options, GFP_KERNEL);
+ if (!options) {
+ P9_DPRINTK(P9_DEBUG_ERROR,
+ "failed to allocate copy of option string\n");
+ return -ENOMEM;
+ }
+
while ((p = strsep(&options, ",")) != NULL) {
int token;
if (!*p)
continue;
token = match_token(p, tokens, args);
if (token < Opt_uname) {
- ret = match_int(&args[0], &option);
- if (ret < 0) {
+ int r = match_int(&args[0], &option);
+ if (r < 0) {
P9_DPRINTK(P9_DEBUG_ERROR,
"integer field, but no integer?\n");
+ ret = r;
continue;
}
}
@@ -139,6 +147,13 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses)
case Opt_access:
s = match_strdup(&args[0]);
+ if (!s) {
+ P9_DPRINTK(P9_DEBUG_ERROR,
+ "failed to allocate copy"
+ " of option argument\n");
+ ret = -ENOMEM;
+ break;
+ }
v9ses->flags &= ~V9FS_ACCESS_MASK;
if (strcmp(s, "user") == 0)
v9ses->flags |= V9FS_ACCESS_USER;
@@ -158,6 +173,7 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses)
}
}
kfree(options);
+ return ret;
}
/**
@@ -173,6 +189,7 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
{
int retval = -EINVAL;
struct p9_fid *fid;
+ int rc;
v9ses->uname = __getname();
if (!v9ses->uname)
@@ -191,7 +208,18 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
v9ses->dfltuid = V9FS_DEFUID;
v9ses->dfltgid = V9FS_DEFGID;
v9ses->options = kstrdup(data, GFP_KERNEL);
- v9fs_parse_options(v9ses);
+ if (!v9ses->options) {
+ P9_DPRINTK(P9_DEBUG_ERROR,
+ "failed to allocate copy of option string\n");
+ retval = -ENOMEM;
+ goto error;
+ }
+
+ rc = v9fs_parse_options(v9ses);
+ if (rc < 0) {
+ retval = rc;
+ goto error;
+ }
v9ses->clnt = p9_client_create(dev_name, v9ses->options);
--
1.5.4.2.134.g82883