Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero

[Matthew Wilcox <matthew@wil.cx>]

On Fri, Apr 15, 2005 at 10:03:05PM +1000, Herbert Xu wrote:
> I suppose it could be smart and stay quiet about
> 
> val < 0 || val > BOUND
> 
> However, gcc is slow enough as it is without adding unnecessary
> smarts like this.

It only warns with -W on, not with -Wall, so I see no compelling
reason to fix this.  I think the real problem here is that 'arg'
is declared 2 pages earlier in the function prototype (aka the
function-growth-hormone-imbalance syndrome).

There's two good ways of fixing this, adding a f_setsig() function:

static inline int f_setsig(struct file *filp, unsigned long arg)
{
	if (arg > _NSIG)
		return -EINVAL;

	filp->f_owner.signum = arg;
	return 0;
}
...
	case F_SETSIG:
		err = f_setsig(filp, arg);
		break;

or add a function that checks a variable to see if it's a valid signal number:

#define valid_signal(arg)	((unsigned long)arg <= _NSIG)
...
	case F_SETSIG:
		if (!valid_signal(arg))
			break;
		err = 0;
		filp->f_owner.signum = arg;
		break;

Looks like futex.c, ptrace.c, signal.c, sys.c and almost every
architecture's ptrace code could easily make use of the latter, but not
the former.  It also looks like we have a few off-by-one errors.  For
example, in h8300's ptrace code:

                case PTRACE_SYSCALL:
                case PTRACE_CONT: {
                        ret = -EIO;
                        if ((unsigned long) data >= _NSIG)
                                break ;

but

                case PTRACE_SINGLESTEP: {
                        ret = -EIO;
                        if ((unsigned long) data > _NSIG)
                                break;

so I'd recommend the second solution.  But be careful not to "fix up"
cases like:

./kernel/exit.c:        if (sig < 1 || sig > _NSIG)

where we really don't want to allow zero.

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/