[Linux-ha-dev] Re: [Linux-ha-cvs] Linux-HA CVS: heartbeat by alan from 24.221.212.80

[Lars Marowsky-Bree]

On 2004-09-10T01:12:23,
   linux-ha-cvs at lists.community.tummy.com said:

I think the CVS commit commentaries are helpful. At least they help me
think about the code going in, so one day I can pretend to know what's
going on in the various bits ;-)

> linux-ha CVS committal
> 
> Author  : alan
> Host    : 24.221.212.80
> Project : linux-ha
> Module  : heartbeat
> 
> Dir     : linux-ha/heartbeat
> 
> 
> Modified Files:
> 	config.c findif.c hb_api.c hb_resource.c heartbeat.c 
> 
> 
> Log Message:
> BEAM CHANGES: Fixed a couple of very minor bugs, and cleaned up some BEAM warnings.
> 
> ===================================================================
> RCS file: /home/cvs/linux-ha/linux-ha/heartbeat/config.c,v
> retrieving revision 1.125
> retrieving revision 1.126
> diff -u -3 -r1.125 -r1.126
> --- config.c	31 Aug 2004 20:58:21 -0000	1.125
> +++ config.c	10 Sep 2004 01:12:23 -0000	1.126
> @@ -1,4 +1,4 @@
> -/* $Id: config.c,v 1.125 2004/08/31 20:58:21 alan Exp $ */
> +/* $Id: config.c,v 1.126 2004/09/10 01:12:23 alan Exp $ */
>  /*
>   * Parse various heartbeat configuration files...
>   *
> @@ -1774,14 +1774,17 @@
>  	char	loginname[64];
>  	struct passwd*	pwd;
>  
> -	if (isdigit(loginname[0])) {
> -		return atoi(loginname);
> -	}
>  	if (llen >= (int)sizeof(loginname)) {
> +		cl_log(LOG_ERR 
> +		,	"user id name [%s] is too long", loginname);
>  		return -1;
>  	}
>  	strncpy(loginname, lname, llen);
>  	loginname[llen] = EOS;
> +
> +	if (isdigit(loginname[0])) {
> +		return atoi(loginname);
> +	}

This doesn't introduce a new bug, but if we have this check, we probably
should verify that the whole string is all digits. Yes, there are
broken setups out there which have usernames in the form of '5dd', we
should at least cl_log(LOG_WARN,...) about that and proceed to call
getpwnam.

>  	if ((pwd = getpwnam(loginname)) == NULL) {
>  		cl_log(LOG_ERR 
>  		,	"Invalid user id name [%s]", loginname);

And if we check for the existence of the name, it may be helpful to use
getpwuid() for the number too.

> @@ -2023,6 +2026,9 @@
>  
>  /*
>   * $Log: config.c,v $
> + * Revision 1.126  2004/09/10 01:12:23  alan
> + * BEAM CHANGES: Fixed a couple of very minor bugs, and cleaned up some BEAM warnings.
> + *
>   * Revision 1.125  2004/08/31 20:58:21  alan
>   * Fixed the size of a memset call...
>   *
> ===================================================================
> RCS file: /home/cvs/linux-ha/linux-ha/heartbeat/findif.c,v
> retrieving revision 1.38
> retrieving revision 1.39
> diff -u -3 -r1.38 -r1.39
> --- findif.c	29 Apr 2004 14:18:20 -0000	1.38
> +++ findif.c	10 Sep 2004 01:12:23 -0000	1.39
> @@ -1,4 +1,4 @@
> -/* $Id: findif.c,v 1.38 2004/04/29 14:18:20 kevin Exp $ */
> +/* $Id: findif.c,v 1.39 2004/09/10 01:12:23 alan Exp $ */
>  /*
>   * findif.c:	Finds an interface which can route a given address
>   *
> @@ -425,13 +425,13 @@
>  		}else{
>  			unsigned long	bits = atoi(netmaskbits);
>  
> -			if (bits < 1 || bits > 32) {
> +			if (bits < 1 || bits > 31) {

Uhm, what is wrong with a /32 netmask? That one is clearly allowed and
translates to a 255.255.255.255 mask.

(Now a bits <= 8 is more likely to be wrong, but also 'legal'.)

>  				fprintf(stderr
>  				,	"Invalid netmask specification [%s]"
>  				,	netmaskbits);
>  				usage();
> +				return;
>  			}
> -
>  			bits = 32 - bits;
>  			*netmask = (1L<<(bits))-1L;
>  			*netmask = ((~(*netmask))&0xffffffffUL);


Sincerely,
    Lars Marowsky-Brée <lmb at suse.de>

-- 
High Availability & Clustering	   \\\  /// 
SUSE Labs, Research and Development \honk/ 
SUSE LINUX AG - A Novell company     \\//