For the interim and until it is needed, this is most prudently
simply disabled.
FIXES 131890
(cherry picked from commit 22481ab3705522ac1988b7de038c4dbc1e5009a9)
Change-Id: I612ea4a1c85fdb895acc6a71801e659869e849c2
Our ngroups management (since PAGs are still encoded as 2 groups) needs
to ensure that we do not overflow what we are prepared to handle,
and do not panic due to misheld mutexes if we have to return an error
when handling it.
FIXES 131878 (CVE-2015-3286)
(cherry-picked from commit 8ce4a3905268385d3f5a0e11f20594875467cae6)
Change-Id: I34a298ab776f69d52c8f621f79aafc79199f9cc4
MRAFS added the FsCmd pioctl for passing messages to the fileserver;
a bug causes it to write into the wrong memory and potentially panic
clients.
FIXES 131896 (CVE-2015-3285)
(cherry picked from commit ef671f497e9161ec2759446d594789495d3346f1)
Change-Id: I1ee1fa7dff1d2594cfe9fab5ae0b7fc9245803de
Avoid leaking data in pioctl interchange buffers; clear the memory
when one is allocated.
FIXES 131892 (CVE-2015-3284)
(cherry picked from commit 592a99d6e693bc640e2bdfc2e7e5243fcedc8f93)
Change-Id: I90fef404978bd7aae3eb88836bcd4f95587fd45c
bos defaults to not requiring crypt in a lot of cases, instead using clear.
As the simplest way to secure the channel is to enable crypt, do so.
FIXES 131782 (CVE-2015-3283)
(cherry picked from commit 62926630a82b8635d1cb1514b852f9f7a2609311)
Change-Id: Ib9e2514c4d14a77eead69677da1dabf86e526ebc
The CPageWrite flag was originally added to prevent a scenario
where a thread doing "writepage" would realize that the cache
was too full and that some of its contents need to be written
back to the server. Before writing back it would ask the OS to
flush any dirty VM associated with the vcache entries that are
to be written, to make sure the data is not stale. This flush
could itself trigger writeback, leading to deadly recursion.
One such scenario is a process doing mmap writes to a file larger
than the cache.
With some kernel versions and some callers of writepage, this
can cause the mapping to be marked as being in an error state,
leading to EIO errors passed back to user space.
Make the recursion check more specific to only bail when the
calling thread is one that was originally seen writing. A list
of current writers is maintained instead of a single state flag.
This lets other threads (like the flusher thread) go on with
writeback to the same file, and limits the WRITEPAGE_ACTIVATE
return case to call sites that can deal with it.
In testing this helps avoid EIO errors when writing large
chunks of data through mmap.
Thanks to Yadav Yadavendra for extensive analysis and testing.
Reviewed-on: http://gerrit.openafs.org/11124
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 95b857399d71cb1f6619e625bff256f8c4c72c6a)
Change-Id: I08ced97c4f58f95375fda2ed9c707cdf7657e493
Reviewed-on: http://gerrit.openafs.org/11877
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
We no longer have to define a ->write operation, and we can't
expect the underlying disk cache filesystem to have one. Use
the new __vfs_read/write helpers that will select the operation
to use based on what's available for that particular filesystem.
Reviewed-on: http://gerrit.openafs.org/11849
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 5c1237432edf4600111845d175c92252430d5f76)
Change-Id: I21bca85637e07d0e03ef471896d0454eeef68a14
Reviewed-on: http://gerrit.openafs.org/11873
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Make the test here a bit more specific. do_sync_read no longer
exists, but we don't use it for new kernels. Trying to define it
here in terms of generic_file_read is not helpful as that doesn't
exist anymore.
Reviewed-on: http://gerrit.openafs.org/11848
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit fcfa5ae2468d878db962a93d6013fcd3042e6c13)
Change-Id: I87bf0fc856d244d15bdae300f0cd6b80ecb63797
Reviewed-on: http://gerrit.openafs.org/11872
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
The current version of BreakUpPath is slightly broken, since
commit 4e68282e26b0c4569d25d076d54274f0da47a691 -- it has two
output parameters but takes only one length parameter for the
size of the output buffers passed in. The callers ended up using
the shorter of the buffer lengths in question, so there is not
a risk of a buffer overrun, but long paths would not be properly
handled.
There is not really any need to pass in a length at all, since
what is going on is conceptually strdup, and there is no real
need to use strlcpy at all. Make the change from strlcpy to
str(n)dup, and adjust callers to free the outputs as appropriate.
While here, convert writeFile() to use goto and a cleanup handler
to avoid leaks.
Reviewed-on: http://gerrit.openafs.org/11874
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
(cherry picked from commit b8648dbefb3968329d20cad8976ce15947428678)
Change-Id: I13d4ffd4ef46cbb7423d229cf6a6e85f7a42ecc1
Reviewed-on: http://gerrit.openafs.org/11879
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Windows file paths can use either '\' or '/' as a path
separator. libafscp on the other hand requires '/' and argv[0]
will always use '\'.
Introduce a new function ConvertAFSPath() which converts the
input path to '/' and converts \\afs to /afs. A future commit
should access the registry and make use of the NetbiosName and
MountRoot values to perform the conversion correctly.
Reviewed-on: http://gerrit.openafs.org/8430
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 0bef3159d3698a941154bd21352eb35bd94edb8c)
Change-Id: Id174313afae9c64a27133477e43e13731c0ac6b7
Reviewed-on: http://gerrit.openafs.org/11878
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
clang now expects that strlcpy will always be used to prevent overflow
of the destination string, and gives a warning if the size parameter is
based solely on the length of the source string.
Modify the BreakUpPath function so that it takes the size of the
destination string as an argument, and uses this to limit the amount of
data pasted into it.
Reviewed-on: http://gerrit.openafs.org/7086
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
(cherry picked from commit 4e68282e26b0c4569d25d076d54274f0da47a691)
Change-Id: I31003fa06fc331a6313ca17840dcb46f61299921
Reviewed-on: http://gerrit.openafs.org/11845
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
A = malloc(strlen(B)+ 1);
memset(A, 0, strlen(B) + 1);
strlcpy(A, B, strlen(B) + 1);
can be more simply written as
A = strdup(B);
Doing so also avoids a warning from clang that strlcpy isn't checking
for A overflowing.
Reviewed-on: http://gerrit.openafs.org/7077
Reviewed-by: Derrick Brashear <shadow@dementix.org>
Tested-by: Derrick Brashear <shadow@dementix.org>
(cherry picked from commit 1e30c00e7d9b45d65e819d39414939f2d5f7631b)
Change-Id: I886bef77fdedb63d1c83e657c25d112e0a635db2
Reviewed-on: http://gerrit.openafs.org/11844
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
clang 3.5 is more aggressive about these checks than the previous
FreeBSD system compiler, so new warnings (which became errors)
appeared on FreeBSD 11-CURRENT.
In afs_dcache.c, checking &tdc->f for NULL-ness has no effect.
The struct fcache f member of struct dcache is an ordinary structure
element; its address will be the value of tdc plus the offset of
f within struct dcache, which will not be NULL even if tdc is NULL.
In ubik_db_if.c, udbHandle is a file-scope global and thus has
allocated storage; the address of a member variable will never
be NULL. The 0 it was compared against was spelled RX_SECIDX_NULL,
which shows the intended check, which is for the value of the
uh_scIndex member variable, not its address.
In afscp_server.c, srv->conns can never be NULL since conns is a member
variable of struct afscp_server (of array type, containing pointers
to struct rx_connection). Comparing the array member variable against
NULL is comparing the address of the array, which is never NULL since
it is not allocated separately from struct afscp_server.
In fssync-debug.c, state.vop->partName is never NULL because
common_volop_prolog always allocates for state.vop, and the
partName member variable of struct fssync_state is of array type,
and thus is not separately allocated from the containing structure.
Reviewed-on: http://gerrit.openafs.org/11739
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit fb499c2406450fa5dc423a0b038266d3b8e79e33)
Change-Id: I13799a3362508672136f8c603eabdfc0f3ee072d
Reviewed-on: http://gerrit.openafs.org/11843
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Commit 5b3c1042969daec38ccb260e61d665eda0c713ea changed/removed some
uses of abs() on unsigned time values. While the previous use of abs()
was indeed incorrect, the result wasn't necessarily much better, even
though it built with recent compilers, since it only checked for skew
in one direction.
Define and use a macro to correctly evaluate the conditionals in 64-bit
precision, avoiding C's integer promotion rules which prefer unsigned types
(Date) to signed types of the same width (time_t on 32-bit systems).
Reviewed-on: http://gerrit.openafs.org/11850
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 810f0ccd0354dac30af024ca7b5acf3ebabf5f4b)
Change-Id: I29337e1ecd410fcf7733408287930c50c055ff90
Reviewed-on: http://gerrit.openafs.org/11863
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
abs(3) is a function of one variable of type int returning int.
labs(3) is a function of one variable of type long returning long.
labs(3) should be used when the input is of type long, as in
kaprocs.c.
Calling anything from the abs(3) family on a variable of unsigned
type is a bogus type pun, and a logical operation which is a no-op.
(Unsigned values are never negative and thus the absolute value
function is the identity over the entire range of values representable
in an unsigned type.) Just remove the use of abs() for unsigned
values, as in kaprocs.c, krb_udp.c, and vldb_check.c
While in kaprocs.c, wrap a long line that was touched for the
conversion to labs(3), spell the argument to time(3) as NULL
instead of 0, remove unneeded parentheses, and correct the spelling
of "reserved".
Reviewed-on: http://gerrit.openafs.org/11745
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 5b3c1042969daec38ccb260e61d665eda0c713ea)
Change-Id: I82038e41346479dad39466907b95f2d7540f6258
Reviewed-on: http://gerrit.openafs.org/11842
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
When copying into the 'cell' array, use the size of cell, not
the size of some other array that is copied into a few lines previously.
(cherry picked from commit 777870da86e9ed66756e01e858a54e959959482d)
Change-Id: Ib8b523901dd8008038c5a95a7c315b899cff8cee
Reviewed-on: http://gerrit.openafs.org/11807
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-on: http://gerrit.openafs.org/11808
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
If a DISK_UpdateInterfaceAddr RPC is received the server that sent
it restarted. Force the urecovery code to verify the server state.
Change-Id: I465863dc3a52d844b56d576bd55229435556cfd6
Reviewed-on: http://gerrit.openafs.org/11738
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 30667a5d7b86b29f9aafe2b490d89b8e01bfc541)
Reviewed-on: http://gerrit.openafs.org/11774
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
When processing an DISK_Begin RPC verify that there is an active quorum
and that the local database is current. Otherwise, fail the RPC with
a UNOQUORUM error.
The returned error must be UNOQUORUM instead of USYNC becase the returned
error code will be returned by the coordinator's ContactQuorum_iterate()
to the client that triggered the write transaction. Most ubik clients
will only retry if the error is UNOQUORUM.
FIXES 131997
Change-Id: Icaa30e6aca82e7e7d33e9171a4f023970aba61df
Reviewed-on: http://gerrit.openafs.org/11689
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit d47beca13236c64ed935fabeff9d1001e8a8871f)
Reviewed-on: http://gerrit.openafs.org/11773
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Apparently, ancient versions of afsd did not start the check server
daemon (AFSOP_START_CS). The afs_Daemon tries to detect when the check
server daemon is not running and issues a warning to upgrade afsd. The
afs_Daemon waits for the cache initialization to complete (AFSOP_GO)
before detecting if the cache server daemon is started.
Unfortunately, when running with memcache, the cache initialization is
fast enough to race with the start of the check server daemon, and the
"Please install afsd with check server daemon" message is sometimes
printed to the syslog.
Since all modern versions of afsd do start the check server daemon, this
error message is no longer needed, so just remove the message and the
flag used to print it on only once.
Reviewed-on: http://gerrit.openafs.org/11602
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 8ce37d0d4aa4e6107f79efaf5027f31ea5a17604)
Change-Id: I292052c9ba629c85ddc4b76c4b3db7d54ce1d852
Reviewed-on: http://gerrit.openafs.org/11680
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
We have previously documented that volumes over 2TB can result in
inaccuracies, but this documentation does not say how the 'partition'
field in "fs listquota" can be inaccurate. It is confusing to see a
usage of 0% for a partition that you know is being used, so try to
briefly explain in what way this field is inaccurate.
The reason we _under_-report the partition usage is that the
fileserver actually gives back PartBlocksAvail and PartMaxBlocks (not
"blocks used" and "blocks total"). So 1TB used and 4TB total is
truncated to 2TB and given back as 2TB free and 2TB total. One we hit
3TB used we'll report it as 1TB free 2TB total (50%) when the actual
usage is 75%.
Reviewed-on: http://gerrit.openafs.org/11245
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit cd8f24d9a1ba8563c6bef2b8d30885a753e8d30c)
Change-Id: I2bd72cca994414a88073d26d44bef49e9cac3be1
Reviewed-on: http://gerrit.openafs.org/11626
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Chas Williams <3chas3@gmail.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Update configure version strings for 1.6.11.1. Note that macos kext
can be of form XXXX.YY[.ZZ[(d|a|b|fc)NNN]] where d dev, a alpha,
b beta, f final candidate so we have no way to represent 1.6.11.1.
Switch to 1.6.12 dev 1 for macos.
Change-Id: Id8975b3d0dfa17e5bc9357b2fb8090bcbd497a6e
Reviewed-on: http://gerrit.openafs.org/11819
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
The backing_dev_info is only stored in the super_block now.
Reviewed-on: http://gerrit.openafs.org/11756
Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 5cca05d1a1c9883e33c953b31c4cb32252474f77)
Change-Id: Id6f2f2dcc111c46ce1339595e145891758c46bab
Reviewed-on: http://gerrit.openafs.org/11761
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
In an age where Linux version numbers are determined by Google+ polls,
it’s clear that they aren’t going to be very useful for marking major
API compatibility boundaries like they were in the days of 2.2/2.4.
Reviewed-on: http://gerrit.openafs.org/11755
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit a5b091e1ec69d4a43d6f1b1efc93134ef7ed2167)
Change-Id: I5b0da6b43e3cbf5d9a6fa883a09deccb359e53e9
Reviewed-on: http://gerrit.openafs.org/11760
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
The new clang imported for FreeBSD 10.1 has stopped accepting
this argument as a no-op. Fix the kernel module build by
stopping passing it on the compiler command line.
Change-Id: I5557f88abafbfa825213003107830ceac98346ff
Reviewed-on: http://gerrit.openafs.org/11809
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Update all remaining copies of CellServDB in the tree, and make the
Red Hat packaging use it by default too.
Reviewed-on: http://gerrit.openafs.org/11764
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 8cb4a42496f71c3d47ebe30a96ec33478e203c82)
Change-Id: I77854bb86ae0d17c4094705e80586b53ba504956
Reviewed-on: http://gerrit.openafs.org/11767
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
Even though we hope no one will actually ever need to use it, explain
this configure switch in the same place as the others.
Reviewed-on: http://gerrit.openafs.org/11749
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
(cherry picked from commit e02b852f05c0927d48b6467872378bae99df2760)
Change-Id: I4f7e42730afab2387d343880acc81d28b70ef5e9
Reviewed-on: http://gerrit.openafs.org/11752
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
We use some uio structures that were allocated on the stack, but we
only initialize them by initializing individual fields. On some
platforms (Solaris is one known example, but probably not the only
one), there are additional fields we do not initialize. Since we
cannot be certain of what any additional fields there may be, just
zero the whole thing.
This is basically the same change as
I0eae0b49a70aee19f3a9ec118b03cfb3a6bd03a3, but in the rx subtree.
Reviewed-on: http://gerrit.openafs.org/11711
Tested-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
(cherry picked from commit a762e6871ad6837ee126cec9e63d99388b4bf119)
Change-Id: Ie6a2cce500d6a0a7a09c305296f4b34d122d3108
Reviewed-on: http://gerrit.openafs.org/11714
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
d_splice_alias now drops the inode reference on error, so we
need to grab an extra one to make sure that the inode doesn't
go away, and release it when done if there was no error.
For kernels that may not drop the reference, provide an
additional iput() within an ifdef. This could be hooked up
to a configure option to allow building a module for a kernel
that is known not to drop the reference on error. That hook
is not provided here. Affected kernels should be the early
3.17 ones (3.17 - 3.17.2); 3.16 and older kernels should not
return errors here.
[kaduk@mit.edu add configure option to control behavior, which
is mandatory on non-buildbot linux systems]
Reviewed-on: http://gerrit.openafs.org/11643
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Laß <lass@mail.uni-paderborn.de>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 15260c7fdc5ac8fe9fb1797c8e383c665e9e0ccd)
Change-Id: I288eb66c38386fcd6bae0da111d97e211cc5c995
Reviewed-on: http://gerrit.openafs.org/11694
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
In several places in the code, we allocate a 'struct uio' on the
stack, or allocate one from non-zeroed memory. In most of these
places, we initialize the structure by assigning individual fields to
certain values. However, this leaves any remaining fields assigned to
random garbage, if there are any additional fields in the struct uio
that we don't know about.
One such platform is Solaris, which has a field called uio_extflg,
which exists in Solaris 11, Solaris 10, and possibly further back.
One of the flags defined for this field in Solaris 11 is UIO_XUIO,
which indicates that the structure is actually an xuio_t, which is
larger than a normal uio_t and contains additional fields. So when we
allocate a uio on the stack without initializing it, it can randomly
appear to be an xuio_t, depending on what garbage was on the stack at
the time. An xuio_t is a kind of extensible structure, which is used
for things like async I/O or DMA, that kind of thing.
One of the places we make use of such a uio_t is in afs_ustrategy,
which we go through for cache reads and writes on most Unix platforms
(but not Linux). When handling a read (reading from the disk cache
into a mapped page), a copy of our stack-allocated uio eventually gets
passed to VOP_READ. So VOP_READ for the cache filesystem will randomly
interpret our uio_t as an xuio_t.
In many scenarios, this (amazingly) does not cause any problems, since
generally, Solaris code will not notice if something is flagged as an
xuio_t, unless it is specifically written to handle specific xuio_t
types. ZFS is one of the apparent few filesystem implementations that
can handle xuio_t's, and will detect and specially handle a
UIOTYPE_ZEROCOPY xuio_t differently than a regular uio_t.
If ZFS gets a UIOTYPE_ZEROCOPY xuio_t, it appears to ignore the uio
buffers passed in, and supplies its own buffers from its cache. This
means that our VOP_READ request will return success, and act like it
serviced the read just fine. However, the actual buffer that we passed
in will remain untouched, and so we will return the page to the VFS
filled with garbage data.
The way this typically manifests is that seemingly random pages will
contain random data. This seems to happen very rarely, though it may
not always be obvious what is going on when this occurs.
It is also worth noting that the above description on Solaris only
happens with Solaris 11 and newer, and only with a ZFS disk cache.
Anything older than Solaris 11 does not have the xuio_t framework
(though other uio_extflg values can cause performance degradations),
and all known non-ZFS local disk filesystems do not interpret special
xuio_t structures (networked filesystems might have xuio_t handling,
but they shouldn't be used for a cache).
Bugs similar to this may also exist on other Unix clients, but at
least this specific scenario should not occur on Linux (since we don't
use afs_ustrategy), and newer Darwin (since we get a uio allocated for
us).
To fix this, zero out the entire uio structure before we use it, for
all instances where we allocate a uio from the stack or from
non-zeroed memory. Also zero out the accompanying iovec in many
places, just to be safe. Some of these may not actually need to be
zeroed (since we do actually initialize the whole thing, or a platform
doesn't have any additional unknown uio fields), but it seems
worthwhile to err on the side of caution.
Thanks to Oracle for their assistance on this issue, and thanks to the
organization experiencing this issue for their patience and
persistence.
1.6 note: This differs noticeably from the master commit in two
places:
- src/afs/NBSD/osi_vnodeops.c: On master there is no stack-allocated
uio struct here.
- src/afs/VNOPS/afs_vnop_write.c and afs_vnop_read.c: On master,
these code paths are structured quite differently, and are handled
in afs_osi_uio.c instead.
Reviewed-on: http://gerrit.openafs.org/11705
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
(cherry picked from commit 5ef1de5eddccce0e7b135bb9ed4decaa62fc19ce)
Change-Id: I8dbf60637774dff81ff839ccd78f58b3b1e85c5b
Reviewed-on: http://gerrit.openafs.org/11713
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Currently we pass a caller_context_t* to some of Solaris' VFS
functions (VOP_SETATTR, VOP_READ, VOP_WRITE, VOP_RWLOCK,
VOP_RWUNLOCK), but the pointer we pass is to uninitialized memory.
This code was added in commit 51d76681, and this particular argument
is mentioned in
<https://lists.openafs.org/pipermail/openafs-info/2004-March/012657.html>,
where the author doesn't really know what the argument is for.
Over 10 years later, it's still not obvious what this argument does,
since I cannot find any documentation for it. However, browsing
publicly-available Illumos/OpenSolaris source suggests this is used
for things like non-blocking operations for network filesystems, and
is only interpreted by certain filesystems in certain codepaths.
In any case, it's clear that we're not supposed to be passing in an
uninitialized structure, since the struct has actual members that are
sometimes interpreted by lower levels. Other callers in
Illumos/OpenSolaris source seem to just pass NULL here if they don't
need any special behavior. So, just pass NULL.
I am not aware of any issues caused by passing in this uninitialized
struct, and browsing Illumos source and discussing the issue with
Oracle engineers suggest there would currently not be any issues with
the cache filesystems we would be using.
However, it's always possible that issues could arise from this in the
future, or there are issues we don't know about. Any such issues would
almost certainly appear to be non-deterministic and be a nightmare to
track down. So just pass NULL, to avoid the potential issues.
Reviewed-on: http://gerrit.openafs.org/11704
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
(cherry picked from commit b9647ac1062509d6a3997ca575ab1542d04677a2)
Change-Id: I5d247cfa6ada3773d20e3938957dcc31c8664bb2
Reviewed-on: http://gerrit.openafs.org/11712
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
When a corrupt directory is discovered, scanning stops immediately and
readdir returns ENOENT. Currently, the vcache lock is unlocked and the
dcache containing the directory is released, but that's not enough.
It's also necessary to unlock the dcache, on which we hold a read lock,
and to clear the vcache state which records an in-progress readdir.
Reviewed-on: http://gerrit.openafs.org/9971
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@your-file-system.com>
(cherry picked from commit a2bfa0e853fb4954fd06a00ea6fbc55c39e76b8a)
Change-Id: Id648822cd45087e5c0d320e8cfdcd6659bb33559
Reviewed-on: http://gerrit.openafs.org/11707
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
check_bad_parent dereferences vcp->mvid, assuming it is not NULL (vcp
is a root vcache here, so mvid refers to the parent fid). However, in
some situations, vcp->mvid can be NULL.
When we first afs_GetVCache the fid, we try to set mvid by setting
mvid to the 'dotdot' structure in the volume struct. But we get that
volume struct from afs_GetVolume, which can fail (at the very least,
this can fail on network failure when looking up vldb information). If
it fails, then we do not set the mvid parent. On future lookups for
the fid, afs_GetVCache will return early for a fastpath, if the vcache
is already in memory. So, mvid will never get set in such a situation.
We also set the mvid parent fid in afs_lookup if we resolved a
mountpoint to the root vcache. However, this is skipped if CMValid is
not set on the vcache, so if CMValid is cleared right after resolving
the mountpoint (say, perhaps done by some other thread e.g. a callback
break or other reasons), then the mvid parent fid will not be set.
To avoid crashing in these situations, if vcp->mvid is NULL in
check_bad_parent, don't check the mvid, and assume it does not match
(since we don't know what it is).
This is a 1.6-only change. On the master branch, the problem is
addressed by commit 69e84bd8a2ccc6065c3a06b7239c855207b92e4d .
Change-Id: I3550cf5a01811ede17d74770161326667a6e8628
Reviewed-on: http://gerrit.openafs.org/11616
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Add param header files and other config info to provide
support for OpenBSD 5.4.
Reviewed-on: http://gerrit.openafs.org/11130
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit e1d0342326d11a14e1fb0075fb62cc6be9389b97)
Change-Id: Ibdde973cf2a542fc89eba025b7210f8b0c843b95
Reviewed-on: http://gerrit.openafs.org/11700
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
With kernel 3.19 struct nameidata becomes opaque. As a result
we cannot rely on STRUCT_NAMEIDATA_HAS_PATH being true for
new kernels.
Rework the conditions here so that STRUCT_NAMEIDATA_HAS_PATH
is only tested when we're using a nameidata structure and
the result matters.
Also modify a configure test to use a nameidata pointer
instead of an actual structure.
Reviewed-on: http://gerrit.openafs.org/11648
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 72e22eb00f641f137f7dbe4195d6d82f4a8addc9)
Change-Id: Ia794d9006a054d16a3b9e5b8ced55c798244d4c7
Reviewed-on: http://gerrit.openafs.org/11662
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Back in kernel 2.6 .20 struct file lost its f_dentry field
which was replaced by f_path.To ease transition f_dentry
was defined as f_dpath.dentry in the same header.This
define finally gets removed with kernel 3.19.
Keep using f_dentry in the code, but add a configure test
for the presence of f_path and the absence of the f_dentry
macro so we can add it if its missing.
Change - Id:I8e8a7e4d3ddd861018de50af1eb7315e730ad529
Reviewed-on: http://gerrit.openafs.org/11646
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit f9ca302b7a10ffc36f2439e068333ab147791c5a)
Change-Id: I179bf2fbc22e824e40c60c59e5d223d49343e7a5
Reviewed-on: http://gerrit.openafs.org/11660
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
The fields in struct dentry are re-arranged so that d_alias
shares space wth d_rcu inside the d_u union. Some references
need to change from d_alias to d_u.d_alias.
The kernel change was introduced for 3.19 but was also backported
to the 3.18 stable series in 3.18.1, so this commit is required
for 3.19 and current 3.18 kernels.
Reviewed-on: http://gerrit.openafs.org/11642
Reviewed-by: Anders Kaseorg <andersk@mit.edu>
Reviewed-by: Michael Laß <lass@mail.uni-paderborn.de>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit d6f29679098aff171e69511823b340ccf28e5c31)
Change-Id: Ifb6199aa7fa922e64540d9fad1d2d79facbb9761
Reviewed-on: http://gerrit.openafs.org/11659
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Move the Linux specific bit of code to reset the root to
afs/LINUX platform specific files. Things that play with
the Linux vfs internals should not be exposed here.
No functional change, but this helps cleanup some ifdef
mess.
Reviewed-on: http://gerrit.openafs.org/11641
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Laß <lass@mail.uni-paderborn.de>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
(cherry picked from commit 6ca324e565c34d9d04f3c553b7d0febe675ae538)
Change-Id: I82803669dd34d7abeb29040fbb38ec2f000f2601
Reviewed-on: http://gerrit.openafs.org/11658
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Reviewed-by: Daria Brashear <shadow@your-file-system.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
