Messages in this thread | | | Date | Thu, 4 Jun 2009 14:01:24 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: A bug in read operation for /dev/zero and a proposed fix. |
| |
On Thu, 4 Jun 2009, Andrew Morton wrote: > > > > To fix this, I propose that when a fatal signal is pending during > > /dev/zero read operation, we simply return and let the user process die. > > Here is a patch that does that. > > > > Signed-off-by: Salman Qazi <sqazi@google.com> > > --- > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > > index 8f05c38..2ffa36e 100644 > > --- a/drivers/char/mem.c > > +++ b/drivers/char/mem.c > > @@ -696,6 +696,11 @@ static ssize_t read_zero(struct file * file, char __user * buf, > > break; > > buf += chunk; > > count -= chunk; > > + /* The exit code here doesn't actually matter, as userland > > + * will never see it. > > + */ > > + if (fatal_signal_pending(current)) > > + return -ENOMEM; > > cond_resched(); > > } > > return written ? written : -EFAULT; > > OK. I think. > > It's presumptuous to return -ENOMEM: we don't _know_ that this signal > came from the oom-killer. It would be better to return -EINTR here.
I don't think the error matters in this case, since we literally only care about fatal signals, but I agree that EINTR is probably better.
That said, it would be even better to basically act as if it was a signal, and do something like
return written ? written : -EINTR;
because that might allow us to simply make it _totally_ interruptible some day. There is nothing that says that /dev/zero shouldn't act like an interruptible file descriptor (like a pipe), and just return a partial read.
If we want to do this for 2.6.30, though, I very much agree with the notion of limiting it to just fatal signals, though.
Linus
| |