Re: [patch] dlm: some checks can underflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 31, 2013 at 12:02:29PM +0300, Dan Carpenter wrote:
> This is a static checker fix.  We have several places here that check
> the upper limit without checking for negative numbers.  One example of
> this is in find_rsb().
> 
> My static checker marks endian data as user controled so.  The
> "ms->m_header.h_length" variable is tagged as user data because it
> starts as little endian and we convert it at the start of
> dlm_receive_buffer().  That means that receive_extralen() returns
> user controlled data which could be negative.  The call tree here is:
> 
> -> dlm_receive_buffer()
>    -> dlm_receive_message()
>       -> _receive_message()
>          -> receive_request()
> 
>             We get "namelen" from receive_extralen(ms);
> 
>             -> find_rsb()
> 
> It's never perfectly clear how invasive to make a fix like this.  Many
> of the changes in the patch are not needed but I wanted to make things
> consistent.

If it's negative, I don't think it would pass the h_length validation
in dlm_process_incoming_buffer(), but I'm not certain...

> -		int lvblen = rc->rc_header.h_length - sizeof(struct dlm_rcom) -
> -			 sizeof(struct rcom_lock);
> +		unsigned int lvblen = rc->rc_header.h_length -
> +			sizeof(struct dlm_rcom) - sizeof(struct rcom_lock);
>  		if (lvblen > ls->ls_lvblen)
>  			return -EINVAL;

Easier to just change that check to

if (lvblen != ls->ls_lvblen)
	return -EINVAL;

Dave
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux