Bug#509167: mdadm: check asprintf() return codes

Neil Brown neilb at suse.de
Fri Dec 19 05:02:13 UTC 2008


I'm not really keen on taking this sort of patch.
It isn't clear that an abort (caused by the assert) is really much
better than just segfaulting normally ... though you do get a message
I guess.
But it makes the code rather ugly.

Maybe if you defined a asprintf_nofail (possibly an inline in mdadm.h)
and called that it would be acceptable.

Thanks,
NeilBrown


On Thursday December 18, kirkland at canonical.com wrote:
> Package: mdadm
> Version: 2.6.7.1-1
> Severity: normal
> Tags: patch
> 
> Hello,
> 
> I have attached a minor patch that we're carrying in Ubuntu, against the
> mdadm-2.6.7.1-1 source.
> 
> This rather trivial patch checks the return codes of 3 asprintf() memory
> allocations, to prevent segfaults on memory-challenged systems.
> 
> Thanks,
> -- 
> :-Dustin
> 
> Dustin Kirkland
> Ubuntu Server Developer
> Canonical, LTD
> kirkland at canonical.com
> GPG: 1024D/83A61194
> --- mdadm-2.6.7.1.orig/Assemble.c
> +++ mdadm-2.6.7.1/Assemble.c
> @@ -29,6 +29,7 @@
>  
>  #include	"mdadm.h"
>  #include	<ctype.h>
> +#include	<assert.h>
>  
>  static int name_matches(char *found, char *required, char *homehost)
>  {
> @@ -384,11 +385,15 @@
>  		st->ss->getinfo_super(st, &info);
>  		c = strchr(info.name, ':');
>  		if (c) c++; else c= info.name;
> -		if (isdigit(*c) && ((ident->autof & 7)==4 || (ident->autof&7)==6))
> +		if (isdigit(*c) && ((ident->autof & 7)==4 || (ident->autof&7)==6)) {
>  			/* /dev/md/d0 style for partitionable */
> -			asprintf(&mddev, "/dev/md/d%s", c);
> -		else
> -			asprintf(&mddev, "/dev/md/%s", c);
> +			int ret = asprintf(&mddev, "/dev/md/d%s", c);
> +			assert(ret >= 0);
> +		}
> +		else {
> +			int ret = asprintf(&mddev, "/dev/md/%s", c);
> +			assert(ret >= 0);
> +		}
>  		mdfd = open_mddev(mddev, ident->autof);
>  		if (mdfd < 0) {
>  			st->ss->free_super(st);
> only in patch2:
> unchanged:
> --- mdadm-2.6.7.1.orig/config.c
> +++ mdadm-2.6.7.1/config.c
> @@ -35,6 +35,7 @@
>  #include	<ctype.h>
>  #include	<pwd.h>
>  #include	<grp.h>
> +#include	<assert.h>
>  
>  /*
>   * Read the config file
> @@ -559,7 +560,8 @@
>  			alert_mail_from = strdup(w);
>  		else {
>  			char *t= NULL;
> -			asprintf(&t, "%s %s", alert_mail_from, w);
> +			int ret = asprintf(&t, "%s %s", alert_mail_from, w);
> +			assert(ret >= 0);
>  			free(alert_mail_from);
>  			alert_mail_from = t;
>  		}
> _______________________________________________
> pkg-mdadm-devel mailing list
> pkg-mdadm-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/pkg-mdadm-devel





More information about the pkg-mdadm-devel mailing list