Hello,

I spotted a few unchecked memory allocation failures early this week
(here's the one that hasn't been taken yet)
  http://article.gmane.org/gmane.linux.kernel/634811
and went looking for more.  I found and fixed seven more:
[Thanks to Markus Armbruster for a rigorous review. ]

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.

Also, with a new match_strlcpy function, one could avoid the
match_strdup call in fs/affs/super.c, and with a new
match_simple_strtoll function, one could avoid a few more
memory allocations.  That's for another day...

Here's the patch:

-----------------------

>From c029d65e38ca25b4a0677d0434689240618430b4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[EMAIL PROTECTED]>
Date: Fri, 8 Feb 2008 15:23:35 +0100
Subject: [PATCH] Handle memory allocation failure.

* arch/alpha/kernel/module.c (module_frob_arch_sections): Handle kcalloc 
failure.
* 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".
* fs/affs/super.c (parse_options): Handle match_strdup failure.
* fs/hfs/super.c (parse_options): Likewise, twice.
* fs/hfsplus/options.c (hfsplus_parse_options): Likewise.
* sound/usb/usbaudio.c (check_hw_params_convention): Handle kcalloc failure.

Signed-off-by: Jim Meyering <[EMAIL PROTECTED]>
---
 arch/alpha/kernel/module.c |    6 ++++++
 fs/9p/v9fs.c               |   40 ++++++++++++++++++++++++++++++++++------
 fs/affs/super.c            |    3 ++-
 fs/hfs/super.c             |    6 ++++--
 fs/hfsplus/options.c       |    3 ++-
 sound/usb/usbaudio.c       |    2 ++
 6 files changed, 50 insertions(+), 10 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;
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);

diff --git a/fs/affs/super.c b/fs/affs/super.c
index 3c45d49..2c2b69e 100644
--- a/fs/affs/super.c
+++ b/fs/affs/super.c
@@ -198,7 +198,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;
@@ -232,6 +231,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;
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);
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);
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.35.g3cfc
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to