Messages in this thread | | | Date | Fri, 15 Apr 2005 14:21:45 +0100 | From | Matthew Wilcox <> | Subject | Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero |
| |
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/
| |