lkml.org 
[lkml]   [2004]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH][2/2] ide-tape: small cleanups - handle copy_to|from_user() failures
On Mon, 29 Nov 2004, Bill Davidsen wrote:

> Alan Cox wrote:
> > On Sul, 2004-11-28 at 16:32, Jesper Juhl wrote:
> >
> > > #endif /* IDETAPE_DEBUG_BUGS */
> > > count = min((unsigned int)(bh->b_size -
> > > atomic_read(&bh->b_count)), (unsigned int)n);
> > > - copy_from_user(bh->b_data + atomic_read(&bh->b_count), buf,
> > > count);
> > > + if (copy_from_user(bh->b_data + atomic_read(&bh->b_count),
> > > buf, count))
> > > + return -EFAULT;
> > > n -= count;
> > > atomic_add(count, &bh->b_count);
> > > buf += count;
> >
> >
> > If you do this then you don't fix up tape->bh for further operations.
> > Have you tested these changes including the I/O errors ?
>
> But (a) do you have enough information to backout or fixup correctly? And (b)
> won't this result in the whole i/o being treated as invalid?
>
That was my original thought "if copy_from_user fails then something
really bad is happening and we should just fail the entire IO operation".
Then when Alan pointed out that I'd probably be messing up tape->bh I got
nervous becourse it seemed to me that he probably had a point, but when I
went back and looked at the code again I ended up with the conclusion that
even if we did mess it up it wouldn't actually matter since we'll be
failing the entire IO since I also added code in the caller to test for
the <0 return from this function and abort the entire operation in that
case.
Alan: Would you mind explaining why this is not safe? If there's something
I'm missing I'd really like to know.

--
Jesper Juhl


-
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/

\
 
 \ /
  Last update: 2005-03-22 14:08    [from the cache]
©2003-2011 Jasper Spaans