Simon Wilkinson [Thu, 21 Feb 2013 20:36:19 +0000 (20:36 +0000)]
libadmin: Fix a lot of dead assignments in vsprocs
Tidy up a lot of places where we initialise a variable, then
immediately assign a proper value to it, or store a return value
that we don't actually care about.
Simon Wilkinson [Wed, 20 Feb 2013 11:17:41 +0000 (11:17 +0000)]
volser: Don't assign code when we don't care
When we're cleaning up temporary volumes, we don't care whether
it succeeds or not. Don't assign code to the results of these
volume deletions, only to then discard it.
Simon Wilkinson [Tue, 19 Feb 2013 17:22:08 +0000 (17:22 +0000)]
fs: Free parent_dir later in lsmount and flushmount
If lsmount or flushmount encounter an error, then they may include
the contents of parent_dir in their error message. However, in both
cases, this was freed a couple of lines earlier.
Just move the free() later, so that the contents of this variable
are still available.
Simon Wilkinson [Tue, 19 Feb 2013 17:15:42 +0000 (17:15 +0000)]
fstrace: Avoid accessing icl log after zapping it
The for loop in icl_EnumerateLogs looks up the next pointer in the
current entry after zapping it. Depending on reference counts, this
may result in us looking up freed memory.
Take a copy of the next point before zapping the current entry, just
in case.
Simon Wilkinson [Fri, 22 Feb 2013 16:54:17 +0000 (16:54 +0000)]
Unix CM: Fix byte accounting for storebehind
In the current version of CacheStoreDCaches, the stored variable is
maintained within the for loop that iterates over the chunk list. This
means that it is reset to 0 each time we handle a new chunk.
However, this means that our progress is no longer accurately tracked,
as (bytes - stored) no longer gives the number of bytes which remain to
be transfered. In fact, as stored is zeroed with each loop iteration,
(bytes - stored) == bytes. This means that store behind is no longer
activated according to the users settings.
Prior to commit 334114ac58b0039ae90d7e29fa2f019fe068bd79, the
stored variable was maintained within the outer, function, scope.
Just move it back there to restore the previous behaviour.
Simon Wilkinson [Thu, 21 Feb 2013 22:15:11 +0000 (22:15 +0000)]
Unix CM: Don't zero args on dcache failure
Even if allocating the dcache fails, there's no point zeroing the
parameters to the allocation function, as those changes aren't
visible outside of that function.
Jeffrey Altman [Sun, 16 Dec 2012 17:42:17 +0000 (12:42 -0500)]
Windows: Direct IO Support for Service
This patchset implements and enables by default the new
Direct IO pathway between the AFS redirector and the afsd_service.exe.
When Direct IO is enabled all reads and writes are performed by the
AFS redirector locking memory allocated by the kernel and mapping it into
the service's memory address space.
The service supports cache bypass in this mode when the
AFS_REQUEST_FLAG_CACHE_BYPASS flag is set in the request from the
redirector. When cache bypass is active, the AFSCache file is ignored and
data is either directly fetched from or stored to the file server. Cache
bypass is enabled by IIS and other applications that request no
intermediate buffering when opening file handles. This is often done
because the application implements its own data caching. All cache bypass
store operations are synchronous.
When cache bypass is not enabled, the memory region provided by the AFS
redirector is either used to populate the cm_buf_t objects or is populated
by them. When cache bypass is not enabled, one outstanding store
operation can be in flight asynchronously to improve performance.
Direct IO is enabled by default and can be disabled by creating the
registry value.
Peter Scott [Fri, 25 Jan 2013 05:46:37 +0000 (00:46 -0500)]
Windows: Direct IO for AFS Redirector
Implement a new IO processing model in which extents are not passed between
afsredirlib.sys and afsd_service.exe. Instead the AFSCache file is
maintained exclusively by the service and the redirector locks kernel
memory, maps it into the service's address space, and permits the service
to manage all IO directly.
This interface adds an AFS Cache Bypass option to the AFS Redirector which
is activated when the file handle has been opened with the no intermediate
buffering option.
This patchset implements the kernel interface. A subsequent
patchset will implement the service component.
Assisted by Jeffrey Altman <jaltman@your-file-system.com>
Change-Id: I25a4764db060b3b3f2b0de4006479dd3a220c6eb
Reviewed-on: http://gerrit.openafs.org/9210 Reviewed-by: Peter Scott <pscott@kerneldrivers.com> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Jeffrey Altman [Fri, 25 Jan 2013 07:21:27 +0000 (02:21 -0500)]
Windows: Add flag BUF_GET_FLAG_BUFCREATE_LOCKED
Modify the buf_Get() and buf_GetNewLocked() interfaces to
permit the cm_scache.bufCreateLocked lock to be held prior to
calling to buf_Get(). Holding the cm_scache.bufCreateLocked lock
before the buf_Get() call prevents a race with another thread
that attempts to set the file size.
Peter Scott [Fri, 15 Feb 2013 13:44:06 +0000 (08:44 -0500)]
Windows: Permit direct to service non-wildcard lookups
The AFS redirector has required that directories be fully enumerated
when the directory object is opened. This is a very expensive
operation involving large numbers of file server RPC round trips for
directories with tens of thousands of objects and those containing
symlinks.
This patchset delays directory enumeration for the last component
in a path until such time as dirctory data is requested by the
application. If the request is for a non-wildcard pattern, the
service will be asked to provide the details for just the one required
object.
Delaying the directory enumeration improves performance for
GetFileAttributes[Ex], GetDiskFreeSpace[Ex], GetVolumeInformation,
and GetNamedSecurityInfo Win32 API calls. In those cases it is
no longer necessary to enumerate the target directory at all.
Change-Id: I7ef2fbafff925697d8b40e56837ef53bfcc78542
Reviewed-on: http://gerrit.openafs.org/9118 Reviewed-by: Peter Scott <pscott@kerneldrivers.com> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Jeffrey Altman [Tue, 19 Feb 2013 04:11:27 +0000 (23:11 -0500)]
Windows: EvalByName pass LastComponent flag
Add AFS_REQUEST_FLAG_LAST_COMPONENT flag for use with
AFS_REQUEST_TYPE_EVAL_TARGET_BY_NAME requests to the service.
When set the service will perform cm_Lookup calls without the
CM_FLAG_CHECKPATH flag set.
Change-Id: I47ec2fb8b1e2699f2d87a6625b1db549ecb4e03d
Reviewed-on: http://gerrit.openafs.org/9133 Reviewed-by: Peter Scott <pscott@kerneldrivers.com> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Jeffrey Altman [Tue, 19 Feb 2013 02:26:23 +0000 (21:26 -0500)]
Windows: EvaluateByName support case-insensitive lookups
Directory lookups in AFS should favor case-sensivite matches
but permit case-insensitive matches otherwise. The service
should not follow mount points. The redirector exposes mount
points as junctions.
Rod Widdowson [Tue, 19 Feb 2013 16:12:26 +0000 (16:12 +0000)]
Windows: Move work item queues over to the Control Device
Currently, when the library is unloaded it stops all worker
threands and then evaporates the work item queues. Thus
any work items which are pending will disappear.
Whilst it is OK that the threads going away, any work items need to
remain queued so that when the library is restarted the work
can continue. This checkin does this by moving the work item
queues and their synchronization primitives into the FS maintained
Control Device Object Extension. The list of worker threads
remains in the Library Device Object Extension.
Change-Id: If5c7cd3bdfea1a368c8df69649e627bac3a9585f
Reviewed-on: http://gerrit.openafs.org/9139 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Peter Scott <pscott@kerneldrivers.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Rod Widdowson [Mon, 18 Feb 2013 15:33:29 +0000 (10:33 -0500)]
Windows: Call CcDeferWrite rather than loop
If we are about to write into the cache and we do not have enough
memory we call CcDeferWrite and return STATUS_PENDING. This allows
the cache to call us back when there is memory.
The write is performed on the IO queue which is shared wth paging
writes. However this does not cause paging writes to block in a
memory shortage situation since the request will either be deferred
again (releasing a thread to service a paging write) or will complete
quickly. Further we allocate all our resources upfront so we fail
fast and in the appropriate place.
Change-Id: I4efbc14a97d3b34236643973f1f8f85c7ea194a6
Reviewed-on: http://gerrit.openafs.org/9127 Reviewed-by: Rod Widdowson <rdw@steadingsoftware.com> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Peter Scott <pscott@kerneldrivers.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
We never get non cached and yet paging IO. Even if we did it would
be inappropriate to call CcCanIWrite. Therefore, collapse two if
statements into one.
Change-Id: I95c9030836e4f7dc4f7867a8b8b09b97bf57b429
Reviewed-on: http://gerrit.openafs.org/9125 Reviewed-by: Rod Widdowson <rdw@steadingsoftware.com> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Peter Scott <pscott@kerneldrivers.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Simon Wilkinson [Tue, 19 Feb 2013 17:09:15 +0000 (17:09 +0000)]
afsio: Don't leak memory on GetVenusFidBy* failure
The GetVenusFid functions all allocate the fid structure immediately
upon entry to the function. When we return with an error, that structure
is never freed.
Update the call sites so that we don't leak this memory.
Simon Wilkinson [Tue, 19 Feb 2013 17:53:11 +0000 (17:53 +0000)]
libafscp: Actually return callback from FindCallback
Fix FindCallback so that it actually returns the callback that it
found. This requires changing the function prototype so that the
third parameter is passed by reference, and updating the single
call site.
Simon Wilkinson [Tue, 19 Feb 2013 17:30:14 +0000 (17:30 +0000)]
libafscp: Don't free bogus ptr in ResolvPathFromVol
afscp_ResolvPathFromVol makes a copy of the path passed to it using
strdup. It then iterates across that, removing initial '/' characters.
However, this iteration means that 'p' no longer points to the start
of the allocated memory - when we free 'p', we may actually be freeing
an offset into the block, which will make malloc unhappy.
Make a copy of the result from strdup, and use that to free the block.
Simon Wilkinson [Tue, 19 Feb 2013 15:46:52 +0000 (15:46 +0000)]
ptserver: Tidy malloc handling in readpwd
Tidy up the malloc handling in readpwd, so that we don't leak memory
if the user specifies multiple -c arguments. Also avoid assuming that
free(NULL) will always work.
Simon Wilkinson [Tue, 19 Feb 2013 14:44:14 +0000 (14:44 +0000)]
vos: aserver is private
The server specified on the command line is used directly to
initialise the attributes structure. Move the variable so it's
local to the block which uses it, and remove the function-wide
initialiser.
Simon Wilkinson [Tue, 12 Feb 2013 12:59:08 +0000 (12:59 +0000)]
auth: Avoid double free in key parsing
There was an error path whilst reading an extended key file which could
result in a key being freed using free(key), and then freed again
through the afsconf_typedKey_put() mechanism. Remove this double free.
Simon Wilkinson [Fri, 15 Feb 2013 16:35:08 +0000 (16:35 +0000)]
rxgen: Remove pointless assignment
The value we assign to defp is never used (it's almost immediately
overwritten), and it is guaranteed to be the same as the existing
value. So, just remove the assignment.
Simon Wilkinson [Fri, 15 Feb 2013 16:05:33 +0000 (16:05 +0000)]
rxgen: Fix NULL pointer dereference
Avoid a NULL pointer dereference if strchr doesn't find any occurence
of '*' in the string. Whilst we handle the not found case when inserting
a mid string terminator, we don't handle it when restoring the string to
its previous value.
Simon Wilkinson [Fri, 15 Feb 2013 11:55:37 +0000 (11:55 +0000)]
Fix incorrect sizeof() arguments in allocations
In a number of places we have
struct X *val;
val = malloc(sizeof(struct Y));
If sizeof(struct Y) < sizeof(struct X) this is obviously dangerous,
but it is incorrect regardless of the relative sizes of the
structures. Fix all of the occurences of this that clang points out
to us.
Simon Wilkinson [Fri, 15 Feb 2013 16:23:16 +0000 (16:23 +0000)]
aklog: Don't reference freed node whilst deleting
Because deletion is implemented using a for loop, the step of the
loop that moves us to the next node references freed memory when
we've deleted an element. Fix this by just shortcircuiting the
return from the function so we immediately exit.
Simon Wilkinson [Tue, 19 Feb 2013 14:41:26 +0000 (14:41 +0000)]
vos: Remove unused 'done' loop variable
In SendFile we break at the same time as setting the done flag,
so its value is never checked. Just remove it as it is redundant
with the current loop logic.
Simon Wilkinson [Fri, 15 Feb 2013 22:34:36 +0000 (22:34 +0000)]
libadmin: Don't try to release garbage connection
In bos_ServerOpen, initalise the contents of bos_server structure
to 0 using calloc, so that if we jump to the error handling stuff
before they are assigned real values we don't end up trying to
release garbage.
Simon Wilkinson [Tue, 12 Feb 2013 14:07:10 +0000 (14:07 +0000)]
usd: Can't call usd_FileStandard* with NULL
It doesn't make sense to call usd_FileStandard{Input,Output} with
a NULL usd_handle_t (and doing so would crash later in the
function), so don't check for attempts to do so.
Simon Wilkinson [Tue, 12 Feb 2013 13:15:16 +0000 (13:15 +0000)]
vlserver: bulkaddrs are unsigned ints
bulkaddrs_val is a pointer to an array of unsigned ints, not to
an array of ints. Fix the sizeof() used in the call to malloc to silence
a clang warning.
Simon Wilkinson [Tue, 12 Feb 2013 13:12:47 +0000 (13:12 +0000)]
ptserver: Simplify malloc assignment to shut up clang
Using a temporary variable of type (char *) to store the results
of malloc and realloc, and then casting the tmp variable to the
real type causes clang-analyzer to complain. Just simplify this
code by always using the real type in order to shut it up.
Simon Wilkinson [Fri, 15 Feb 2013 17:12:45 +0000 (17:12 +0000)]
viced: Avoid clang errors with modeBits
The modeBits element of the VnodeDiskObject structure is defined as
a 12 bit wide bitfield. This causes clang some problems when doing
integer arithmetic, as it appears to the compiler that the field is
being overflowed. For example...
targetptr->disk.modeBits &= ~04000;
Produces the error:
implicit truncation from 'int' to bitfield changes value
from -2049 to 2047
Marc Dionne suggested changing this to
targetptr->disk.modeBits = targetptr->disk.modeBits & ~04000;
Simon Wilkinson [Fri, 15 Feb 2013 17:08:45 +0000 (17:08 +0000)]
Fix warnings-as-errors for clang
It seems like some versions of clang have a problem with using
pragmas to stop particular warnings being converted to errors with
-Werror. These compilers require that the warning be ignored completely
in order to suppress it.
Make the necessary changes to afsd and bozo, and update README.WARNINGS
to note the problem.
Andrew Deason [Mon, 18 Feb 2013 01:34:06 +0000 (19:34 -0600)]
rx: Assert call error for RXS_PreparePacket error
If we've received an error from the underlying security class, we must
not try to send the given packet, or we risk security issues. We
currently achieve this by setting an error on the connection. It is
slightly indirect in how this yields an error on this specific call,
and so it may not be immediately clear, but doing so is critical. If
somehow the call does not have an error by the end of this, we cannot
proceed as this is an error condition we do not handle. So, assert.
Andrew Deason [Mon, 14 Jan 2013 18:45:04 +0000 (12:45 -0600)]
rx: Honor RXS_PreparePacket errors
rxi_PrepareSendPacket calls RXS_PreparePacket to allow the security
class to modify the given packet appropriately (to be undone by
CheckPacket on the other endpoint). However, currently
rxi_PrepareSendPacket ignores all errors generated by
RXS_PreparePacket, and processing continues as if there was no error.
For rxkad, an error often results in the given packet being untouched.
This means that the security checksum is not calculated, and thus not
populated in the packet, and for encrypted connections means that the
packet contents are not encrypted.
This occurs for any error generated by the security class
PreparePacket routine. For rxkad, the most common error is probably
RXKADEXPIRED, though some other internal errors are possible as well.
This behavior has a few effects for rxkad:
1. When any error is generated by PreparePacket, the other endpoint
generally bails out with the error RXKADSEALEDINCON, since the
security checksum of the packet is 0, which does not match what the
checksum should be. This results in error messages like 'rxk: sealed
data inconsistent'. This can be very confusing if the actual error
is, say, just that the given credentials have expired.
2. For connections requiring encryption (rxkad_crypt), an error from
PreparePacket means that the packet payload is sent in the clear.
This can happen for about a window size's worth of packets.
3. If a client ignores errors/inconsistencies with the checksum and
encryption, etc, they can keep reading data for the call forever,
even after their credentials have expired.
To fix this, make an error from RXS_PreparePacket cause a connection
error for the given connection, and immediately send a connection
abort. No further error checking should be necessary for the callers
of rxi_PrepareSendPacket, since they already check for call/conn
errors before sending any actual packets.
Marc Dionne [Sun, 17 Feb 2013 18:29:38 +0000 (13:29 -0500)]
tests: Improve failure mode for unresolvable hostname
In the case of a host where gethostbyname is unable to resolve
the hostname, afstest_BuildTestConfig() may return NULL which
can cause several tests to crash.
Add a common function to look out for this condition and use it where
appropriate. When it occurs, the current module is skipped and
the user gets an error message that indicates the configuration
problem.
Jeffrey Altman [Thu, 14 Feb 2013 19:43:58 +0000 (14:43 -0500)]
Windows: Ensure pResultCB exists before Authentication
When processing requests from the redirector it is possible for
the ResultCB to not be allocated. This can occur either due to
an out of memory condition in a synchronous request or due to
an asynchronous extents or byte range lock request.
Move the assignment of the Authenticated state after the allocation
of the ResultCB from the stack in case of out of memory conditions.
Jeffrey Altman [Thu, 14 Feb 2013 14:26:16 +0000 (09:26 -0500)]
Windows: Unique file ID is per volume
The unique file ID returned as part of the BY_HANDLE_FILE_INFORMATION
data structure obtained via GetFileInformationByHandle() is only
guarranteed to be unique within the volume where volume uniqueness is
determined by the volume's serial number.
It therefore doesn't make sense to return the volume id as part of
FILE_INTERNAL_INFORMATION IndexNumber. Instead return Vnode and
Unique as that is what ensures uniqueness within an existing AFS
volume.
Unfortunately, {VolId, Vnode, Unique} does not guarantee uniqueness
for when multiple cells are in use.
Jeffrey Altman [Thu, 7 Feb 2013 21:53:45 +0000 (16:53 -0500)]
Windows: RXAFS_BulkStat failures
The RXAFS_BulkStat RPC is quite brain dead. The client requests
status information on up to AFSCBMAX FIDs. The file server replies
success only if all of the client credentials provide access to
all of the requested FIDs. If status info cannot be provided
for any one of the FIDs, the error code of the failure is returned
with no context as to which FID failed.
To simplify the logic within the cache manager a new local error
code, CM_ERROR_BULKSTAT_FAILURE is introduced to replace whatever
error was received from the file server. This error is returned
by cm_TryBulkStat and cm_TryBulkStatRPC. The caller of either of
those functions should interpret the error to mean that the current
user context cannot be used to perform a bulkstat operation against
the provided cm_scache directory. Instead, individual RXAFS_FetchStatus
operations must be performed.
This patchset implements such error handling for both the SMB and
RDR interfaces. This change permits the Windows cache manager to
properly enumerate a directory for which the user only has list
permission and cannot read the status info for files and symlinks.
Jeffrey Altman [Tue, 12 Feb 2013 21:32:18 +0000 (16:32 -0500)]
Windows: cm_BPlusDirNextEnumEntry return all errors
Return all entries in the directory enumeration regardless of any
errors returned from cm_BPlusDirEnumBulkStatNext(). Set the error
code in the returned cm_direnum_entry_t.errorCode field so that
the caller can determine how the error should be handled on a
per entry basis.
Jeffrey Altman [Wed, 13 Feb 2013 19:04:28 +0000 (14:04 -0500)]
Windows: Add Cell name to AFSProcessRequest parameters
Knowing the cell name for the request can be useful to the file
system driver which otherwise does not have access to a conversion
from FileID.CellID to Cell name.