afs: Avoid incorrect size when fetching beyond EOF 28/11828/3
authorAndrew Deason <adeason@sinenomine.net>
Fri, 10 Apr 2015 00:58:51 +0000 (19:58 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Mon, 25 Jan 2016 01:27:07 +0000 (20:27 -0500)
commit3caee7575491c33920b9c84f5dc4098d99373cf6
tree6b87a8345f8d22d8ce6f718f18a42168b04035d8
parent4a69d3894c68a52f592b43d3d8d8cb73691f7eee
afs: Avoid incorrect size when fetching beyond EOF

Currently, afs_GetDCache contains a couple of calculations that look
similar to this:

    if (position + size > file_length) {
        size = file_length - position;
    }
    if (size < 0) {
        size = 0;
    }

Most of the time, this is fine. However, if 'position' is more than
2GiB greater than file_length, 'size' will calculated to be smaller
than -2GiB. Since 'size' in this code is a signed 32-bit integer, this
can cause 'size' to underflow, and result in a value closer to
(positive) 2GiB.

This has two potential effects:

The afs_AdjustSize call in afs_GetDCache will cause the underlying
cache file for this dcache to be very large (if our offset is around
2GiB larger than the file size). This can confuse other parts of the
client, since our cache usage reporting will be incorrect (and can be
even way larger than the max configured cache size).

This will also cause a read request to the fileserver that is larger
than necessary. Although 'size' will be capped at our chunksize, it
should be 0 in this situation, since we know there is no data to
fetch. At worst, this currently can just result in worse performance
in rare situations, but it can also just be very confusing.

Note that an afs_GetDCache request beyond EOF can currently happen in
non-race conditions on at least Solaris when performing a file write.
For example, with a chunksize of 256KiB, something like this will
trigger the overflow in 'size' in most cases:

    $ printf '' > smallfile && printf b | dd of=smallfile bs=1 oseek=2147745793

But there are probably other similar scenarios.

To fix this, just check if our offset is beyond the relevant file
size, and do not depend on 'size' having sane values in edge cases
such as this.

Change-Id: Ie36f66ce11fbee905062b3a787871ec077c15354
Reviewed-on: http://gerrit.openafs.org/11828
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
src/afs/afs_dcache.c