Benjamin Kaduk [Sun, 22 Nov 2015 19:24:43 +0000 (13:24 -0600)]
volser: set error, not code, before rfail
The rfail cleanup handler overwrites 'code' ~unconditionally, but
does use an existing 'error' value if present. Since the intent
is to return failure to the caller, preserve the code in the error
variable and do so.
Benjamin Kaduk [Mon, 23 Nov 2015 00:22:58 +0000 (18:22 -0600)]
Fix optimized IRIX kernel module builds
Commit 9f94892f8d996a522e7801ef6088a13769bee7c2 (from 2006)
introduced per-file CFLAGS, using $(CFLAGS-$@); this construct
is not parsed well by IRIX make, which ends up attempting to
expand '$@)' and finding mismatched parentheses.
Commit 5987e2923a2670a27a801461dc9668ec88ed7d2a (from 2007) followed,
fixing the IRIX build but only for the NOOPT case. This left the
problematic expression in CFLAGS_OPT until 2013, when another RT
ticket was filed reporting the continued breakage. That ticket
was then ignored until 2015 (now) with no particular cries of
outrage on the mailing lists. Perhaps this gives some indication
of the size and/or mindset of the IRIX userbase. (There have
been successful IRIX installations during this time period, so
presumably it was discovered that disabling optimizations helped
the build along.)
FIXES 131621
Reviewed-on: http://gerrit.openafs.org/12111 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 767694d9ec86fc9451f5a4ba2ec7405c29986a21)
Change-Id: Ie5d349b1e9f8a768efcb461d7367d2d7deac31f6
Reviewed-on: https://gerrit.openafs.org/12198 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Stephan Wiesand [Tue, 8 Mar 2016 13:15:17 +0000 (14:15 +0100)]
Linux 4.4: Do not use splice()
splice() may return -ERESTARTSYS if there are pending signals, and
it's not even clear how this should be dealt with. This potential
problem has been present for a long time, but as of Linux 4.4
(commit c725bfce7968009756ed2836a8cd7ba4dc163011) seems much more
likely to happen.
Until resources are available to fix the code to handle such errors,
avoid the riskier uses of splice().
If there is a default implementation of file_splice_{write,read},
use that; on somewhat older kernels where it is not available,
use the generic version instead.
[kaduk@mit.edu: add test for default_file_splice_write]
Reviewed-on: https://gerrit.openafs.org/12217 Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit ae5f411c3b374367ab8ae69488f78f8e0484ce48)
Change-Id: I40dd0d60caece6379a62674defb8d46a2bfadad6
Reviewed-on: https://gerrit.openafs.org/12228 Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Michael Laß [Mon, 18 Jan 2016 17:29:00 +0000 (18:29 +0100)]
Linux 4.4: key_payload has no member 'value'
In Linux 4.4 (146aa8b1453bd8f1ff2304ffb71b4ee0eb9acdcc) type-specific and
payload data have been merged. The payload is now accessed directly and has
no 'value' member anymore.
FIXES 132677
Reviewed-on: https://gerrit.openafs.org/12169 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Tested-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 5067ee3ae11932a3f1c972c8f88b20afbd9e1d88)
Change-Id: I5a3e89b2676b463935e9a77042cbcd8ab812dc68
Reviewed-on: https://gerrit.openafs.org/12226 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Tested-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Michael Meffie [Fri, 18 Mar 2016 14:22:33 +0000 (10:22 -0400)]
doc: fs examine no longer requires read rights on the volume root vnode
Update the man page to reflect the current access rights required for fs
examine. Historically, fs examine required read access on the root
vnode of the volume housing the directory or file being examined. This
access check was relaxed in commit d2d591caf2c9b4cf2ebae708cc9b4c8b78ca5a5a,
since the information returned by the file server is already available
anonymously by other means.
Reviewed-on: https://gerrit.openafs.org/12223 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit f99c1ec32bb6e8d31ac517173ff7502dbd85aa05)
Change-Id: I580d1e0cab7f823ac1932f99066495cef9e2410a
Reviewed-on: https://gerrit.openafs.org/12224 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Benjamin Kaduk [Wed, 16 Mar 2016 21:16:49 +0000 (16:16 -0500)]
Add param files for FreeBSD 10.2, 10.3
FreeBSD 10.3 is in the beta stage now; better get ready for it.
Reviewed-on: https://gerrit.openafs.org/12222 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 02a393de6b30a500b77f276011c70d41eff363b5)
[updated to match the FreeBSD param.h files on openafs-stable-1_6_x]
Change-Id: Iae290edd29b34aa849f7422b48c765f81eb802fe
Reviewed-on: https://gerrit.openafs.org/12232 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Stephan Wiesand [Tue, 8 Mar 2016 13:15:17 +0000 (14:15 +0100)]
Linux 4.4: Do not use splice()
splice() may return -ERESTARTSYS if there are pending signals, and
it's not even clear how this should be dealt with. This potential
problem has been present for a long time, but as of Linux 4.4
(commit c725bfce7968009756ed2836a8cd7ba4dc163011) seems much more
likely to happen.
Until resources are available to fix the code to handle such errors,
avoid the riskier uses of splice().
If there is a default implementation of file_splice_{write,read},
use that; on somewhat older kernels where it is not available,
use the generic version instead.
[kaduk@mit.edu: add test for default_file_splice_write]
Change-Id: Ib4477cdfb2cd0f49f516da75edc3cb9d1a8817dc
Reviewed-on: https://gerrit.openafs.org/12217 Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Michael Laß [Mon, 18 Jan 2016 17:29:00 +0000 (18:29 +0100)]
Linux 4.4: key_payload has no member 'value'
In Linux 4.4 (146aa8b1453bd8f1ff2304ffb71b4ee0eb9acdcc) type-specific and
payload data have been merged. The payload is now accessed directly and has
no 'value' member anymore.
FIXES 132677
Change-Id: Id26c40c80314a0087ecc0735029412787058ef07
Reviewed-on: https://gerrit.openafs.org/12169 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Tested-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Chas Williams [Mon, 23 Nov 2015 19:15:08 +0000 (14:15 -0500)]
rxgen: Don't use size_t in struct rx_opaque with XDR
OpenAFS's XDR doesn't support size_t at this time. For now, use a
temporary stack variable to avoid 32/64-bit issues and copy back the
returned value upon success.
Change-Id: Ia3dd8abd665a19e04aa611f940728d088a8f87b7
Reviewed-on: https://gerrit.openafs.org/12115 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Michael Meffie [Fri, 18 Mar 2016 14:22:33 +0000 (10:22 -0400)]
doc: fs examine no longer requires read rights on the volume root vnode
Update the man page to reflect the current access rights required for fs
examine. Historically, fs examine required read access on the root
vnode of the volume housing the directory or file being examined. This
access check was relaxed in commit d2d591caf2c9b4cf2ebae708cc9b4c8b78ca5a5a,
since the information returned by the file server is already available
anonymously by other means.
Change-Id: If62b625bce8a260b98fb56a6feec49c674f2de53
Reviewed-on: https://gerrit.openafs.org/12223 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Benjamin Kaduk [Tue, 15 Mar 2016 04:15:20 +0000 (23:15 -0500)]
OPENAFS-SA-2016-002 ListAddrByAttributes information leak
The ListAddrByAttributes structure is used as an input to the GetAddrsU
RPC; it contains a Mask field that controls which of the other fields
will actually be read by the server during the RPC processing.
Unfortunately, the client only wrote to the fields indicated by the
mask, leaving the other fields uninitialized for transmission on the
wire, leaking some contents of client memory.
Plug the information leak by zeroing the entire structure before use.
Benjamin Kaduk [Tue, 15 Mar 2016 04:15:20 +0000 (23:15 -0500)]
OPENAFS-SA-2016-002 VldbListByAttributes information leak
The VldbListByAttributes structure is used as an input to several
RPCs; it contains a Mask field that controls
which of the other fields will actually be read by the server
during the RPC processing. Unfortunately, the client only
wrote to the fields indicated by the mask, leaving the other
fields uninitialized for transmission on the wire, leaking
some contents of client memory.
Plug the information leak by zeroing the entire structure before use.
Benjamin Kaduk [Tue, 15 Mar 2016 04:15:20 +0000 (23:15 -0500)]
OPENAFS-SA-2016-002 AFSStoreVolumeStatus information leak
The AFSStoreVolumeStatus structure is used as an input to the
RXAFS_SetVolumeStatus RPC; it contains a Mask field that controls
which of the other fields will actually be read by the server
during the RPC processing. Unfortunately, the client only
wrote to the fields indicated by the mask, leaving the other
fields uninitialized for transmission on the wire, leaking
some contents of kernel memory.
Plug the information leak by zeroing the entire structure before use.
Benjamin Kaduk [Sun, 13 Mar 2016 17:56:24 +0000 (12:56 -0500)]
OPENAFS-SA-2016-002 AFSStoreStatus information leak
Marc Dionne reported that portions of the AFSStoreStatus structure
were not written to before being sent over the network for
operations such as create, symlink, etc., leaking the contents
of the kernel stack to observers. Which fields in the request
are used are controlled by a flags field, and so if a field was
not going to be used by the server, it was sometimes left
uninitialized.
Fix the information leak by zeroing out the structure before use.
Jeffrey Altman [Thu, 10 Mar 2016 02:38:10 +0000 (20:38 -0600)]
OPENAFS-SA-2016-001 group creation by foreign users
CVE-2016-2860:
In AFS 3.3 as part of the addition of the cross-cell support for foreign
user auto-registration a bug was introduced that permits foreign users
to create arbitrary groups as if they were system administrators. This
permits the groups to be created without any group quota checks, and
using group names that non-administrators would not normally be able to
create, such as groups with the "system:" prefix or groups with no colon
(that is, in the namespace for users).
Additionally, all entries created using the auto-registration service
were marked as being created by system:administrators. This behavior
should not be changed on the stable release branch, but for the next
release the behavior will change to show these entries as being
self-created, to better reflect reality.
FIXES 132822
[kaduk@mit.edu: reword commit message, minor style adjustments]
Jeffrey Altman [Thu, 10 Mar 2016 04:34:55 +0000 (22:34 -0600)]
ptserver: fix pt_util creation of groups
In commit 53ac98931adf9f04c150d9bc084cae31f3913476 the adjustment of
owner id was moved from CreateEntry() into CreateGroupName(). This was
done for two reasons:
1. to reuse the computation of "is administrator" within
CreateGroupName() in order to permit the owner id to be set
to the invalid values 0 and ANONYMOUSID.
2. to allow the owner id to be altered in ChangeEntry().
Unfortunately, CreateEntry() needs to be able to alter the owner id
when creating users not only groups.
This change moves the computation of "is administrator" and the
owner id assignment to CreateEntry() and ChangeEntry().
Benjamin Kaduk [Tue, 15 Mar 2016 04:15:20 +0000 (23:15 -0500)]
OPENAFS-SA-2016-002 ListAddrByAttributes information leak
The ListAddrByAttributes structure is used as an input to the GetAddrsU
RPC; it contains a Mask field that controls which of the other fields
will actually be read by the server during the RPC processing.
Unfortunately, the client only wrote to the fields indicated by the
mask, leaving the other fields uninitialized for transmission on the
wire, leaking some contents of client memory.
Plug the information leak by zeroing the entire structure before use.
Benjamin Kaduk [Tue, 15 Mar 2016 04:15:20 +0000 (23:15 -0500)]
OPENAFS-SA-2016-002 VldbListByAttributes information leak
The VldbListByAttributes structure is used as an input to several
RPCs; it contains a Mask field that controls
which of the other fields will actually be read by the server
during the RPC processing. Unfortunately, the client only
wrote to the fields indicated by the mask, leaving the other
fields uninitialized for transmission on the wire, leaking
some contents of client memory.
Plug the information leak by zeroing the entire structure before use.
Benjamin Kaduk [Tue, 15 Mar 2016 04:15:20 +0000 (23:15 -0500)]
OPENAFS-SA-2016-002 AFSStoreVolumeStatus information leak
The AFSStoreVolumeStatus structure is used as an input to the
RXAFS_SetVolumeStatus RPC; it contains a Mask field that controls
which of the other fields will actually be read by the server
during the RPC processing. Unfortunately, the client only
wrote to the fields indicated by the mask, leaving the other
fields uninitialized for transmission on the wire, leaking
some contents of kernel memory.
Plug the information leak by zeroing the entire structure before use.
Benjamin Kaduk [Sun, 13 Mar 2016 17:56:24 +0000 (12:56 -0500)]
OPENAFS-SA-2016-002 AFSStoreStatus information leak
Marc Dionne reported that portions of the AFSStoreStatus structure
were not written to before being sent over the network for
operations such as create, symlink, etc., leaking the contents
of the kernel stack to observers. Which fields in the request
are used are controlled by a flags field, and so if a field was
not going to be used by the server, it was sometimes left
uninitialized.
Fix the information leak by zeroing out the structure before use.
Benjamin Kaduk [Thu, 10 Mar 2016 01:30:20 +0000 (19:30 -0600)]
OPENAFS-SA-2016-001 group creation by foreign users
CVE-2016-2860:
The ptserver permits foreign-cell users to create groups as if they were
system:administrators. In particular, groups in the user namespace
(with no colon) and the system: namespace can be created. No group
quota is enforced for the creation of these groups, but they will be
owned by system:administrators and cannot be changed by the user that
created them. When processing requests from foreign users, the
creator ID is overwritten with the ID of system:administrators, and
that field is later used for access control checks in
CorrectGroupName(), called from CreateEntry().
The access-control bypass is not possible for creating user entries,
since there is an early check in CreateOK() that only permits
administrators to create users, using a correct test for whether
the call is being made by an administrator.
Brian Torbich [Thu, 21 Jan 2016 15:08:27 +0000 (10:08 -0500)]
redhat: Correct permissions on systemd unit files
Change the systemd unit file permissions created via
openafs.spec to be 0644 instead of 0755. Having the
systemd unit files be executable will trigger a systemd
warning.
FIXES 132662
Reviewed-on: http://gerrit.openafs.org/12174 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit a4c4b786059ac7d5f9ecc5ec07727f000b62c13f)
Stephan Wiesand [Mon, 22 Jun 2015 08:44:11 +0000 (10:44 +0200)]
redhat: Avoid bogus dependencies when building the srpm
By default the spec defines that both userland and kernel module
packages should be built. This results in a dependency of the form
"kernel-devel-`uname -m` = `uname -r`" being added to the source
package created by makesrpm.pl, which is bogus because the uname
values are from the system on which the srpm is built and needn't
apply to the system where it is used. While rpm and rpmbuild ignore
such dependencies of source packages, other tools don't and may fail.
Some versions of rpmbuild will also enforce those requirements when
building the srpm itself, which is pointless too.
Avoid both problems by pretending not to attempt building modules
and ignoring any dependencies when makesrpm.pl invokes rpmbuild -bs.
Reviewed-on: http://gerrit.openafs.org/11903 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Tested-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 9ee5fa152b7b7de6a6ddc6ed87bbf9f76da6e3e4)
Change-Id: I76aac20b8dcad2105f8d20a3e169b2f5526ef956
Reviewed-on: http://gerrit.openafs.org/12195 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
Mark Vitale [Mon, 9 Feb 2015 23:16:16 +0000 (18:16 -0500)]
pioctl.c: restore required result variable
Commit b9fb9c62a6779aa997259ddf2a83a90b08e04d5f refactored lpioctl()
so that LINUX would have its own implementation. This also simplified
the other lpioctl() implementations by removing superfluous variable
'rval'.
Unfortunately, 'rval' was actually required for both DARWIN and SUN511.
On both of these platforms, the address of 'errcode' is passed
to the respective ioctl_*() routine so its value may be passed back
to lpioctl(). Therefore, 'errcode' must not also be used for the
return value from these functions; doing so results in the return
value from the function overwriting the intended value of 'errcode' upon
return to lpioctl().
In the case of Solaris 11, ioctl_sun_afs_syscall() always returns zero
(as long as the ioctl device 'dev/afs' opened successfully).
So 'errcode' was always being set to zero, even if the pioctl had
actually failed. For example, without this fix, 'fs listcells'
loops forever on Solaris 11, listing an infinite number of "cells",
because it will never "see" the EDOM that informs it of the last defined
cell.
Benjamin Kaduk [Thu, 6 Feb 2014 21:11:49 +0000 (16:11 -0500)]
pioctl.c: removed unused variable
The 'rval' variable is only actually used in the LINUX20 case;
adding another conditional block is making the LINUX20 case
different enough that it should get split out entirely.
Doing so lets the 'else' clause be simpler.
Found by clang on FreeBSD 10.0.
Reviewed-on: http://gerrit.openafs.org/10819 Tested-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit b9fb9c62a6779aa997259ddf2a83a90b08e04d5f)
Change-Id: I47f781bc13d54ad5a1b34365fcb9680793b206d1
Reviewed-on: http://gerrit.openafs.org/11778 Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Benjamin Kaduk [Thu, 6 Feb 2014 22:01:19 +0000 (17:01 -0500)]
FBSD: Switch the dummy 'data' for mount(2)
The mount(2) API takes a void*, but 'rn' is const char*, which
is const-incorrect. Our vfs_cmount implementation ignores the 'data'
parameter, but upstream's kernel mount(2) implementation did
have a NULL check until r158611 (in the 6.1 or 7.0 timeframe),
so leave that comment for now.
Arguably we should be using nmount(2) instead of mount(2) anyway,
but leave that for a separate patch.
Michael Meffie [Thu, 7 Jan 2016 19:15:53 +0000 (14:15 -0500)]
Linux: Fix crash when the afs root volume is not found
Commit 602130f1de65eefeb4e31e114070d544eb9edd40 changed the allocation of the
backing device info to directly use the kernel memory allocator. Unfortunately,
one of the deallocations was not converted to the kernel memory deallocator
in the backport to the 1.6.x branch.
The code path is triggered when the afs root volume is not found (for example,
not -dynroot and the root.afs volume is not available.) This causes the system
to crash instead of just failing to mount /afs.
This is a 1.6.x change only. This bug was introduced in version 1.6.14.1.
FIXES 132653
Change-Id: Ifc991be5f914b4a4e1a797b7e2178dc03436b8e6
Reviewed-on: http://gerrit.openafs.org/12166 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Michael Meffie [Wed, 24 Feb 2016 21:57:11 +0000 (16:57 -0500)]
LINUX: ifconfig is deprecated
ifconfig is deprecated and is no longer installed by default on RHEL 7 and
Centos 7. Use the replacement ip command in the init script for linux.
Fallback to ifconfig in the event the ip command is not available.
Thanks to Ben Kaduk for pointing out the hash built-in command.
Change-Id: I7ffe272eb712cd83a70a7d880d239f72b40cb5df
Reviewed-on: http://gerrit.openafs.org/12192 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Note: This crash was exposed by other bugs (to be addressed in future
commits) in OpenAFS large volume support. However, there may
be other failure paths (unrelated to large volumes) that expose
this error as well.
When VAllocVnode() must allocate a new vnode but fails while
updating the vnode index file (e.g. an "addled bitmap" due to other
bugs in working with a vnode index larger than 2^31 bytes), it branches
to common recovery logic at label error_encountered:.
Part of this recovery is to call VFreeBitmapEntry_r(). Commit 08ffe3e81d875b58ae5fe4c5733845d5132913a0 added a VOL_FREE_BITMAP_WAIT
flag to VFreeBitmapEntry() in order to prevent races with VAllocBitmapEntry().
If the caller specifies VOL_FREE_BITMAP_WAIT, VFreeBitmapEntry_r will
call VCreateReservation_r() and VWaitExclusiveState_r(). However, the
exit from VFreeBitmapEntry_r() calls VCancelReservation_r() unconditionally.
This works correctly with the majority of callers to VFreeBitmapEntry_r,
which do specify the VOL_FREE_BITMAP_WAIT flag.
However, the VAllocVnode() error_encountered logic must specify 0 for
this flag because the thread is already in an exclusive state
(VOL_STATE_VNODE_ALLOC). This correctly causes VFreeBitmapEntry_r() to
forgo both the reservation and wait-for-exclusive-state. However, before
exit it erroneously calls VCancelReservation_r(). We now have unbalanced
reservations (nWaiters); this causes an assert when the VAllocVnode()
error_encountered recovery code later calls VCancelReservation_r()
for what it believes is its own prior reservation.
Modify VFreeBitmapEntry_r() to make its final VCancelReservation_r()
conditional on flag VOL_FREE_BITMAP_WAIT.
Change-Id: Id6cf6b1279b11e6dfc4704bba5739912f663beca
Reviewed-on: http://gerrit.openafs.org/11983 Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Jeffrey Altman <jaltman@auristor.com> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Mark Vitale [Sat, 18 Jul 2015 05:12:51 +0000 (01:12 -0400)]
bozo: allow start of fs and dafs bnodes with options
fs_create() and dafs_create() issue stat() to verify
the existence of each executable specified in the bnode.
However, commit fda2bc874751ca479365dc6389c0eebb41a0bda1
inadvertently removed the code that stripped any command
arguments before the stat(). Therefore, any bnode that
specifies arguments (e.g. /usr/afs/bin/dafileserver -d 5),
causes the stat() to fail and the bnode will not start.
Rename function AppendExecutableExtension() to a less
"window-ish" name: PathToExecutable().
Modify the Windows version of PathToExecutable()
to properly strip arguments.
Reimplement the Unix macro as function PathToExecutable()
that properly strips arguments.
Change-Id: I04f7ce2afb8211bd12b9063db1335738bff1cc1e
Reviewed-on: http://gerrit.openafs.org/11934 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Tested-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Michael Meffie [Mon, 8 Feb 2016 15:10:32 +0000 (10:10 -0500)]
test: skip buserror test when SIGBUS is not defined in perl POSIX module
Older versions of the perl POSIX module do not define the SIGBUS symbol, which
causes the opr/softsig-t perl test to fail to compile. Instead of trying to
defined SIGBUS, which may be platform dependent, skip the buserror unit test on
these older platforms.
Michael Meffie [Fri, 30 Jan 2015 17:20:10 +0000 (12:20 -0500)]
volser: detect eof in dump stream while reading acl
Detect an EOF condition while reading the ACL in a dump stream
and return a restore error, instead of filling the ACL with
0xFF and then failing the restore due to an invalid tag.
Benjamin Kaduk [Sun, 22 Nov 2015 20:23:49 +0000 (14:23 -0600)]
cellconfig: check for invalid dotted quads
IP addresses entered into the CellServDB with components larger
than 255 would silently be trucated down to 8-bit unsigned integer
representations. This could cause confusing behavior with
occasional hangs.
FIXES 131794
Change-Id: I44834cb4662e178fdb4be2eeb03ad58d2fa7c556
Reviewed-on: http://gerrit.openafs.org/12109 Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Andrew Deason [Sun, 12 Apr 2015 01:51:09 +0000 (20:51 -0500)]
afs: Log abnormally large chunk files
Any chunk in our cache for a regular file should be smaller than or
equal to our configured chunksize. If someone sets a chunk to be
larger than that, it is very strange and may cause other confusing
issues. Specifically, afs_DoPartialWrite determines if our cache is
"too full" by counting the number of dirty chunks. If we have a dirty
chunk that is much larger than the chunksize, it can throw off the
afs_DoPartialWrite calculation.
This is only true for dcaches backing regular files, though. For
directories, we fetch the entire directory into a single chunk file,
and the size of a directory blob can easily exceed the chunksize
without issues. The aforementioned issue with afs_DoPartialWrite does
not apply, since directory chunks cannot be dirty (we only locally
modify the chunk if we modify the dir on the server, and the DVs
match).
Anyway, it should not be possible to get a chunk for a regular file
larger than the chunksize. Log a message if it does occur, to help
assist anyone in tracking down issues when this does occur.
[mmeffie@sinenomine.net remove unnecessary casts in afs_warn args.]
Change-Id: I5cf58e3659dc04255c62fa56b044d5bc1c7ce877
Reviewed-on: http://gerrit.openafs.org/11831 Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Chas Williams [Sat, 25 Apr 2015 20:38:12 +0000 (16:38 -0400)]
opr: Disable some warnings during opr assertions
Detect _Pragma(), a C99 extension for inline #pragma's, and use it to
disable to certain warnings during the use of opr_Verify() and
opr_Assert().
Because some versions of clang support _Pragma, do not have support
for -Wtautological-pointer-compare, and do set -Werror and -Wunknown-pragmas,
we must explicitly check for pragma support for -Wtautological-pointer-compare
as well.
Change-Id: Id3d5ee347f320a366a0571572b58414aa7044bf7
Reviewed-on: http://gerrit.openafs.org/11852 Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Andrew Deason [Fri, 10 Apr 2015 02:26:25 +0000 (21:26 -0500)]
afs: Log weird 'size' fetchdata errors
There are a couple of situations that should never happen when issuing
a fetchdata, but cause errors when they do:
- The fileserver responds with more than 2^32 bytes of data
- The fileserver responds with more data than requested (but still
smaller than 2^32)
While these should normally never be encountered, it can be very
confusing when they do, since they cause file fetches to fail. To give
the user or investigating developer some hope of figuring out what is
going on, at least log a warning in these situations, to at least
indicate this is the area in which something is breaking.
Only log these once, in case something causes these conditions to be
hit, e.g., every fetch. Once is at least enough to say this is
happening.
[mmeffie@sinenomine.net remove unneeded casts in afs_warn args and
explicit static initializers.]
Andrew Deason [Wed, 8 Apr 2015 03:10:53 +0000 (22:10 -0500)]
afs: Fix fetchInit for negative/large lengths
Currently, the 'length64' variable in rxfs_fetchInit is almost
completely unused (it just goes into an icl logging function). For the
length that we actually use ('*alength'), we just take the lower 32
bits of the length that the fileserver told us. This method is
incorrect in at least the following cases:
- If the fileserver returns a length that is larger than 2^32-1,
we'll just take the lower 32 bits of the 64-bit length the
fileserver told us about. The client currently never requests a
fetch larger than 2^32-1, so this would be an error, but if this
occurred, we would not detect it until much later in the fetch.
- If the fileserver returns a length that is larger than 2^31-1, but
smaller than 2^32, we'll interpret the length as negative (which we
assume is just 0, due to bugs in older fileservers). This is also
incorrect.
- If the fileserver returns a negative length smaller than -2^31+1,
we may interpret the give length as a positive value instead of a
negative one. Older fileservers can do this if we fetch data beyond
the file's EOF (this was fixed in the fileserver in commit 529d487d65d8561f5d0a43a4dc71f72b86efd975). This positive length
will cause an error (usually), instead of proceeding without error
(which is what would happen if we correctly interpreted the length
as negative).
On Solaris, this can manifest as a failed write, when writing to a
location far beyond the file's EOF from the fileserver's point of
view, because Solaris writes can trigger a fetch for the same area.
Seeking to a location far beyond the file's EOF and writing can
trigger this, as can a normal copy into AFS, if the file is large
enough and the cache is large enough. To explain in more detail:
When copying a file into AFS, the cache manager will buffer the dirty
data in the disk cache until the file is synced/closed, or we run out
of cache space. While this data is buffering, the application will
write into an offset, say, 3GiB into the file. On Solaris, this can
trigger a read for the same region, which will trigger a fetch from
the fileserver at the offset 3GiB into the file. If the fileserver
does not contain the fix in commit 529d487d65d8561f5d0a43a4dc71f72b86efd975, it will respond with a large
negative number, which we interpret as a large positive number; much
larger than the requested length. This will cause the fetch to fail,
which then causes the whole write() call to fail. Specifically this
will fail with EINVAL on Solaris, since that is the error code we
return from afs_GetOnePage when we fail to acquire a dcache. If the
cache is small enough, this will not happen, since we will flush data
to the fileserver before we have a large amount of dirty data,
e.g., 3GiB. (The actual error occurs closer to 2GiB, but this is just
for illustrative purposes.)
To fix this, detect the various ranges of values mentioned above, and
handle them specially. Lengths that are too large will yield an error,
since we cannot handle values over 2^31-1 in the rxfs_* framework
currently.
For lengths that are negative, just act as if we received a length of
0. Do this for both the 64-bit codepath and the non-64-bit codepath,
just so they remain identical.
[mmeffie@sinenomine.net: directly use 64 bit comparisons, don't mask
end call error code, commit nits.]
Change-Id: I7e8f2132d52747b7f0ce4a6a5ba81f6641a298a8
Reviewed-on: http://gerrit.openafs.org/11829 Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
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:
Michael Meffie [Thu, 21 Jan 2016 22:55:37 +0000 (17:55 -0500)]
doc: afsd -settime and -nosettime are obsolete
Update the afsd man page -settime and -nosettime options, which are obsolete
and no longer have any effect. Use the same wording as the other obsolete
options in the afsd man page. Keep the recommendations to use the time keeping
daemons provided by the operating system to maintain the system time.
Change-Id: I08a1bd5ae0b2d6618b3e212ebcbb98f470e33820
Reviewed-on: http://gerrit.openafs.org/12175 Reviewed-by: Michael Laß <lass@mail.uni-paderborn.de> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Benjamin Kaduk [Mon, 23 Nov 2015 00:22:58 +0000 (18:22 -0600)]
Fix optimized IRIX kernel module builds
Commit 9f94892f8d996a522e7801ef6088a13769bee7c2 (from 2006)
introduced per-file CFLAGS, using $(CFLAGS-$@); this construct
is not parsed well by IRIX make, which ends up attempting to
expand '$@)' and finding mismatched parentheses.
Commit 5987e2923a2670a27a801461dc9668ec88ed7d2a (from 2007) followed,
fixing the IRIX build but only for the NOOPT case. This left the
problematic expression in CFLAGS_OPT until 2013, when another RT
ticket was filed reporting the continued breakage. That ticket
was then ignored until 2015 (now) with no particular cries of
outrage on the mailing lists. Perhaps this gives some indication
of the size and/or mindset of the IRIX userbase. (There have
been successful IRIX installations during this time period, so
presumably it was discovered that disabling optimizations helped
the build along.)
FIXES 131621
Change-Id: Id5298103221b016239723aa08ebe0dc54bdadc5e
Reviewed-on: http://gerrit.openafs.org/12111 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Chas Williams [Thu, 24 Dec 2015 22:58:32 +0000 (17:58 -0500)]
LINUX: don't cache negative entries for dynroot
The dynroot volume lacks any callbacks that would invalidate the directory
or change the data version. Further, the data version for the dynroot
is only updated for when a new cell is found or added (a positive lookup).
Change-Id: If0b022933de7335d3d94aafc77c50b85b99f4116
Reviewed-on: http://gerrit.openafs.org/12140 Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Marcio Barbosa [Tue, 29 Dec 2015 13:31:43 +0000 (10:31 -0300)]
afs: do not allow two shutdown sequences in parallel
Often, ‘afsd -shutdown’ is called right after ‘umount’.
Both commands hold the glock before calling ‘afs_shutdown’.
However, one of the functions called by 'afs_shutdown', namely,
‘afs_FlushVCBs’, might drop the glock when the global
'afs_shuttingdown' is still equal to 0. As a result, a scenario
with two shutdown sequences proceeding in parallel is possible.
To fix the problem, the global ‘afs_shuttingdown’ is used as an
enumerated type to make sure that the second thread will not run
‘afs_shutdown’ while the first one is stuck inside ‘afs_FlushVCBs’.
Change-Id: Iffa89d82278b0df5fb90fc35608af66d8e8db29e
Reviewed-on: http://gerrit.openafs.org/12016 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Brian Torbich [Thu, 21 Jan 2016 15:08:27 +0000 (10:08 -0500)]
redhat: Correct permissions on systemd unit files
Change the systemd unit file permissions created via
openafs.spec to be 0644 instead of 0755. Having the
systemd unit files be executable will trigger a systemd
warning.
FIXES 132662
Change-Id: I9f5111c855941528193aaabeb42bf1b732246a7e
Reviewed-on: http://gerrit.openafs.org/12174 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Stephan Wiesand [Mon, 22 Jun 2015 08:44:11 +0000 (10:44 +0200)]
redhat: Avoid bogus dependencies when building the srpm
By default the spec defines that both userland and kernel module
packages should be built. This results in a dependency of the form
"kernel-devel-`uname -m` = `uname -r`" being added to the source
package created by makesrpm.pl, which is bogus because the uname
values are from the system on which the srpm is built and needn't
apply to the system where it is used. While rpm and rpmbuild ignore
such dependencies of source packages, other tools don't and may fail.
Some versions of rpmbuild will also enforce those requirements when
building the srpm itself, which is pointless too.
Avoid both problems by pretending not to attempt building modules
and ignoring any dependencies when makesrpm.pl invokes rpmbuild -bs.
Change-Id: I0134e1936638c7d9c3fd9ff0ccf1cba36710d0d3
Reviewed-on: http://gerrit.openafs.org/11903 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Tested-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Benjamin Kaduk [Wed, 4 Feb 2015 15:11:29 +0000 (10:11 -0500)]
Update extra-iput configure argument description
Commit 15260c7fdc5ac8fe9fb1797c8e383c665e9e0ccd did not function
as advertised, since the conditional which attempted to make
the configure option --(en|dis)able-linux-d_splice_alias-extra-iput
mandatory on linux checked a variable for the system type which
was not set at the time the check ran.
Subsequent discussion of this behavior produced a consensus that
there is not a need to make the configure option mandatory,
due to the narrow range of kernels affected by the bug in question,
so this follow-up commit just fixes the documentation and removes
the ineffective code.
Change-Id: I36d1f8801d355f33c3132fcab166ea76faab8e87
Reviewed-on: http://gerrit.openafs.org/11710 Reviewed-by: Anders Kaseorg <andersk@mit.edu> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com>
Simon Wilkinson [Mon, 4 Mar 2013 16:15:37 +0000 (16:15 +0000)]
compile_et: Don't overflow input file buffer
Don't overlow the buffer that's used for the input filename by
copying in to much with sprintf. Use asprintf to dynamically
allocate a buffer instead.
Link roken for rk_asprintf where needed.
Build compile_et with libtool, to ensure that it is linked statically,
as is needed for build tools such as compile_et. (This requires
a preceding change to set a buildtool_roken make variable.)
Benjamin Kaduk [Wed, 25 Feb 2015 23:46:28 +0000 (18:46 -0500)]
Provide a buildtool_roken make variable
When using roken in build tools, i.e., binaries which must be
executed during the build stage, the roken library must be usable
prior to the 'install' stage. In particular, if the internal
rokenafs is used, the shared library will not be installed and
the runtime linker will not be able to find it, causing execution
of the build tool to fail. To avoid this failure, librokenafs
must be linked statically into these build tools.
Unfortunately, the way we currently use libtool is not very
well aligned to libtools model of how it should be used. As a result,
it does not seem feasible to cause libtool to link librokenafs
statically without breaking other parts of the build.
Libtool peeks at the compiler command-line arguments to affect its
behavior when invoked as a linker. The flags -static, -all-static,
and -static-libtool-libs can affect whether dynamic or static linkage
is used for various libraries being linked into the executable.
Passing -all-static tells libtool to not do any dynamic linking at
all, but is silently a no-op if static linking is not possible (the
default situation on most modern Linuxen, OS X, and Solaris).
Passing -static causes libtool to not do any dynamic linking of
libtool libraries which have not been installed, and passing
-static-libtool-libs causes libtool to not do any dynamic linking
of libtool libraries at all.
In order to get libtool to actually link statically in all cases,
we should pass -all-static, not just -static. However, because
too many platforms disallow static linking by default, this is
not a viable option.
If we retain the libtool archive librokenafs.la in the linker search
path, libtool then records the library dependency of libafshcrypto on
librokenafs in its metadata and refuses to install libafshcrypto.la to
any path other than the configured prefix. This restriction of
libtool is incompatible with our use in 'make dest', and it is not
feasible to desupport 'make dest' before the 1.8 release.
The most appropriate workaround seems to be to just pass the
path to librokenafs.a on the linker command line when linking
build tools. As such, provide a new make variable buildtool_roken
which is appropriate for linking roken into build tools -- this
variable will be set to the path to librokenafs.a when the internal
roken is used, and the normal -lrokenafs when an external roken
is used.
Change-Id: I079fc6de5d0aa6403eb1071f3d58a248b1777853
Reviewed-on: http://gerrit.openafs.org/11763 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Anders Kaseorg [Fri, 31 Jul 2015 05:42:55 +0000 (01:42 -0400)]
rxkad: Resolve warnings in ticket5.c
Resolves these warnings:
ticket5.c: In function ‘tkt_MakeTicket5’:
ticket5.c:574:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncTicketPart’ differ in signedness [-Wpointer-sign]
code = encode_EncTicketPart(encodebuf, allocsiz, &data, &encodelen);
^
In file included from ticket5.c:80:0:
v5gen-rewrite.h:43:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
#define encode_EncTicketPart _rxkad_v5_encode_EncTicketPart
^
v5gen.c:1889:1: note: in expansion of macro ‘encode_EncTicketPart’
encode_EncTicketPart(unsigned char *p, size_t len, const EncTicketPart * data, size_t * size)
^
ticket5.c:602:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncryptedData’ differ in signedness [-Wpointer-sign]
code = encode_EncryptedData(ticket + *ticketLen - 1, *ticketLen, &encdata, &tl);
^
In file included from ticket5.c:80:0:
v5gen-rewrite.h:16:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
#define encode_EncryptedData _rxkad_v5_encode_EncryptedData
^
v5gen.c:690:1: note: in expansion of macro ‘encode_EncryptedData’
encode_EncryptedData(unsigned char *p, size_t len, const EncryptedData * data, size_t * size)
^
ticket5.c: In function ‘tkt_DecodeTicket5’:
ticket5.c:320:10: warning: ‘plainsiz’ may be used uninitialized in this function [-Wmaybe-uninitialized]
code = decode_EncTicketPart((unsigned char *)plain, plainsiz, &decr_part, &siz);
^
Marcio Barbosa [Thu, 24 Dec 2015 20:23:23 +0000 (17:23 -0300)]
viced: do not overwrite possible failure
The function ‘hpr_Initialize’ overwrites the code
returned by ‘ubik_ClientInit’. As a result, ‘hpr_Initialize’
will not report any failure triggered by ‘ubik_ClientInit’.
To fix this problem, store the code returned by ‘rxs_Release’
in a new variable. Only return this code if the function
‘ubik_ClientInit’ worked properly. Otherwise, return the code
provided by ‘ubik_ClientInit’.
Stephan Wiesand [Mon, 14 Dec 2015 14:11:37 +0000 (15:11 +0100)]
Update NEWS for 1.6.16
Release notes for OpenAFS 1.6.16
Change-Id: I5c1676b2bad4e94039691fb17f33fb5e278fadbf
Reviewed-on: http://gerrit.openafs.org/12131 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Mark Vitale [Fri, 7 Aug 2015 15:56:16 +0000 (11:56 -0400)]
afs: pioctl kernel memory overrun
CVE-2015-8312:
Any pioctl with an input buffer size (ViceIoctl->in_size)
exactly equal to AFS_LRALLOCSIZE (4096 bytes) will cause
a one-byte overwrite of its kernel memory working buffer.
This may crash the operating system or cause other
undefined behavior.
The attacking pioctl must be a valid AFS pioctl code.
However, it need not specify valid arguments (in the ViceIoctl),
since only rudimentary checking is done in afs_HandlePioctl.
Most argument validation occurs later in the individual
pioctl handlers.
Nor does the issuer need to be authenticated or authorized
in any way, since authorization checks also occur much later,
in the individual pioctl handlers. An unauthorized user
may therefore trigger the overrun by either crafting his
own malicious pioctl, or by issuing a privileged
command, e.g. 'fs newalias', with appropriately sized but
otherwise arbitrary arguments. In the latter case, the
attacker will see the expected error message:
"fs: You do not have the required rights to do this operation"
but in either case the damage has been done.
Pioctls are not logged or audited in any way (except those
that cause loggable or auditable events as side effects).
root cause:
afs_HandlePioctli() calls afs_pd_alloc() to allocate two
two afs_pdata structs, one for input and one for output.
The memory for these buffers is based on the requested
size, plus at least one extra byte for the null terminator
to be set later:
requested size allocated
================= =================================
> AFS_LRALLOCSIZ osi_Alloc(size+1)
<= AFS_LRALLOCSIZ afs_AllocLargeSize(AFS_LRALLOCSIZ)
afs_HandlePioctl then adds a null terminator to each buffer,
one byte past the requested size. This is safe in all cases
except one: if the requested in_size was _exactly_
AFS_LRALLOCSIZ (4096 bytes), this null is one byte beyond
the allocated storage, zeroing a byte of kernel memory.
Commit 6260cbecd0795c4795341bdcf98671de6b9a43fb introduced
the null terminators and they were correct at that time.
But the commit message warns:
"note that this works because PIGGYSIZE is always less than
AFS_LRALLOCSIZ"
Commit f8ed1111d76bbf36a466036ff74b44e1425be8bd introduced
the bug by increasing the maximum size of the buffers but
failing to account correctly for the null terminator in
the case of input buffer size == AFS_LRALLOCSIZ.
Commit 592a99d6e693bc640e2bdfc2e7e5243fcedc8f93 (master
version of one of the fixes in the recent 1.6.13 security
release) is the fix that drew my attention to this new
bug. Ironically, 592a99 (combined with this commit), will
make it possible to eliminate the "offending" null termination
line altogether since it will now be performed automatically by
afs_pd_alloc().
[kaduk@mit.edu: adjust commit message for CVE number assignment,
reduce unneeded churn in the diff.]
Mark Vitale [Fri, 7 Aug 2015 15:56:16 +0000 (11:56 -0400)]
afs: pioctl kernel memory overrun
CVE-2015-8312:
Any pioctl with an input buffer size (ViceIoctl->in_size)
exactly equal to AFS_LRALLOCSIZE (4096 bytes) will cause
a one-byte overwrite of its kernel memory working buffer.
This may crash the operating system or cause other
undefined behavior.
The attacking pioctl must be a valid AFS pioctl code.
However, it need not specify valid arguments (in the ViceIoctl),
since only rudimentary checking is done in afs_HandlePioctl.
Most argument validation occurs later in the individual
pioctl handlers.
Nor does the issuer need to be authenticated or authorized
in any way, since authorization checks also occur much later,
in the individual pioctl handlers. An unauthorized user
may therefore trigger the overrun by either crafting his
own malicious pioctl, or by issuing a privileged
command, e.g. 'fs newalias', with appropriately sized but
otherwise arbitrary arguments. In the latter case, the
attacker will see the expected error message:
"fs: You do not have the required rights to do this operation"
but in either case the damage has been done.
Pioctls are not logged or audited in any way (except those
that cause loggable or auditable events as side effects).
root cause:
afs_HandlePioctli() calls afs_pd_alloc() to allocate two
two afs_pdata structs, one for input and one for output.
The memory for these buffers is based on the requested
size, plus at least one extra byte for the null terminator
to be set later:
requested size allocated
================= =================================
> AFS_LRALLOCSIZ osi_Alloc(size+1)
<= AFS_LRALLOCSIZ afs_AllocLargeSize(AFS_LRALLOCSIZ)
afs_HandlePioctl then adds a null terminator to each buffer,
one byte past the requested size. This is safe in all cases
except one: if the requested in_size was _exactly_
AFS_LRALLOCSIZ (4096 bytes), this null is one byte beyond
the allocated storage, zeroing a byte of kernel memory.
Commit 6260cbecd0795c4795341bdcf98671de6b9a43fb introduced
the null terminators and they were correct at that time.
But the commit message warns:
"note that this works because PIGGYSIZE is always less than
AFS_LRALLOCSIZ"
Commit f8ed1111d76bbf36a466036ff74b44e1425be8bd introduced
the bug by increasing the maximum size of the buffers but
failing to account correctly for the null terminator in
the case of input buffer size == AFS_LRALLOCSIZ.
Commit 592a99d6e693bc640e2bdfc2e7e5243fcedc8f93 (master
version of one of the fixes in the recent 1.6.13 security
release) is the fix that drew my attention to this new
bug. Ironically, 592a99 (combined with this commit), will
make it possible to eliminate the "offending" null termination
line altogether since it will now be performed automatically by
afs_pd_alloc().
[kaduk@mit.edu: adjust commit message for CVE number assignment,
reduce unneeded churn in the diff.]
Chas Williams [Wed, 2 Dec 2015 15:38:42 +0000 (10:38 -0500)]
Open syscall emulation file O_RDONLY
As reported on the -info mailing list, docker is now exporting the
/proc filesystem as read only. ioctl() doesn't need write permissions
to do its work, so change O_RDWR to O_RDONLY.
Reviewed-on: http://gerrit.openafs.org/12122 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 359e1f2a25d242984229edfb378c0b95c3ee8570)
configure now checks for the standard getmaxyx() macro; failing that,
it looks for the older but pre-standardization getmaxx() and getmaxy(),
then falls back to the 4.2BSD curses _maxx and _maxy fields; if all
else fails, gtx building is disabled.
gtx now defines getmaxyx() itself if necessary, based on the above.
This also fixes a bug in gtx with all ncurses versions > 1.8.0 on
platforms other than NetBSD and OS X: gtx was using the _maxx and
_maxy fields, which starting with ncurses 1.8.1 were off by 1 from
the expected values. As such, behavior of scout and/or afsmonitor
may change on most ncurses-using platforms.
Reviewed-on: http://gerrit.openafs.org/12107 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit b800f7d9bd5ea390ab330c1c0c38ac8277eb9998)
Change-Id: Ia42eb33a963aa15131511c07ef4823f3f061a762
Reviewed-on: http://gerrit.openafs.org/12125 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams <3chas3@gmail.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Stephan Wiesand [Tue, 8 Dec 2015 12:13:47 +0000 (13:13 +0100)]
redhat: exclude kpasswd from debuginfo processing
While kpasswd was in the separate openafs-kpasswd package to avoid
clashing with the krb5 executable, openafs-debuginfo still conflicted
with krb5-debuginfo. Remove the x-bits from kpasswd in %install to
make debuginfo processing ignore it, and add them back in the %files
list. Make kapasswd a copy rather than a hard link to have it processed
in the usual way.
This is a 1.6-only change. On the master branch, this issue is fixed
by commit 4e3ceaccd9dc2b6e6a20e938d82af1ebaa2c43c8 which however
removes kpasswd altogether and is thus considered inapproriate for the
stable release series.
FIXES 131771
Change-Id: Icd940e3f5da133a98401c7a28ed6ee0c637bf602
Reviewed-on: http://gerrit.openafs.org/12128 Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Michael Meffie [Fri, 30 Jan 2015 17:12:03 +0000 (12:12 -0500)]
volser: range check acl header fields during dumps and restores
Perform range checks on the acl header fields when reading an
acl from a dump stream and when writing an acl to a dump
stream.
Before this change, a bogus value in the total, positive, or
negative acl fields from a dump stream could cause an out of
bounds access of the acl entries table, crashing the volume
server.
Change-Id: Ic7d7f615a37491835af8d92f3c5f1b6a667d9d01
Reviewed-on: http://gerrit.openafs.org/11702 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Mark Vitale <mvitale@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Benjamin Kaduk [Sun, 22 Nov 2015 19:24:43 +0000 (13:24 -0600)]
volser: set error, not code, before rfail
The rfail cleanup handler overwrites 'code' ~unconditionally, but
does use an existing 'error' value if present. Since the intent
is to return failure to the caller, preserve the code in the error
variable and do so.
Michael Meffie [Mon, 30 Mar 2015 17:20:42 +0000 (13:20 -0400)]
dafs: remove the salvageserver -showlog option
Remove the salvagerserver option to print log messages to stdout. This
was a carry over from the stand-alone salvager and is not appropriate for
a daemon.
projects / packages / o / openafs.git / log