Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero
From: Matthew Wilcox (matthew_at_wil.cx)
Date: 04/15/05
- Previous message: Nick Piggin: "Re: [patch] sched: fix sched domain degenerate"
- In reply to: Herbert Xu: "Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero"
- Next in thread: Jesper Juhl: "Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero"
- Reply: Jesper Juhl: "Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Fri, 15 Apr 2005 14:21:45 +0100 To: Herbert Xu <herbert@gondor.apana.org.au>
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/
- Previous message: Nick Piggin: "Re: [patch] sched: fix sched domain degenerate"
- In reply to: Herbert Xu: "Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero"
- Next in thread: Jesper Juhl: "Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero"
- Reply: Jesper Juhl: "Re: [PATCH] fs/fcntl.c : don't test unsigned value for less than zero"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
