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    [W:0.023 / U:179.720 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site