22 Feb, 2008

1 commit

  • Sorry for the noise, but here's the v3 of this compilation fix :)

    There are some places, which declare the char buf[...] on the stack
    to push it later into dprintk(). Since the dprintk sometimes (if the
    CONFIG_SYSCTL=n) becomes an empty do { } while (0) stub, these buffers
    cause gcc to produce appropriate warnings.

    Wrap these buffers with RPC_IFDEBUG macro, as Trond proposed, to
    compile them out when not needed.

    Signed-off-by: Pavel Emelyanov
    Acked-by: J. Bruce Fields
    Signed-off-by: Trond Myklebust

    Pavel Emelyanov
     

15 Feb, 2008

6 commits

  • d_path() is used on a pair. Lets use a struct path to
    reflect this.

    [akpm@linux-foundation.org: fix build in mm/memory.c]
    Signed-off-by: Jan Blunck
    Acked-by: Bryan Wu
    Acked-by: Christoph Hellwig
    Cc: Al Viro
    Cc: "J. Bruce Fields"
    Cc: Neil Brown
    Cc: Michael Halcrow
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jan Blunck
     
  • seq_path() is always called with a dentry and a vfsmount from a struct path.
    Make seq_path() take it directly as an argument.

    Signed-off-by: Jan Blunck
    Cc: Christoph Hellwig
    Cc: Al Viro
    Cc: "J. Bruce Fields"
    Cc: Neil Brown
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jan Blunck
     
  • I'm embedding struct path into struct svc_expkey.

    Signed-off-by: Jan Blunck
    Cc: Al Viro
    Acked-by: "J. Bruce Fields"
    Cc: Neil Brown
    Cc: Christoph Hellwig
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jan Blunck
     
  • I'm embedding struct path into struct svc_export.

    [akpm@linux-foundation.org: coding-style fixes]
    [ezk@cs.sunysb.edu: NFSD: fix wrong mnt_writer count in rename]
    Signed-off-by: Jan Blunck
    Acked-by: J. Bruce Fields
    Acked-by: Christoph Hellwig
    Cc: Al Viro
    Cc: "J. Bruce Fields"
    Cc: Neil Brown
    Cc: Trond Myklebust
    Signed-off-by: Erez Zadok
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jan Blunck
     
  • * Add path_put() functions for releasing a reference to the dentry and
    vfsmount of a struct path in the right order

    * Switch from path_release(nd) to path_put(&nd->path)

    * Rename dput_path() to path_put_conditional()

    [akpm@linux-foundation.org: fix cifs]
    Signed-off-by: Jan Blunck
    Signed-off-by: Andreas Gruenbacher
    Acked-by: Christoph Hellwig
    Cc:
    Cc: Al Viro
    Cc: Steven French
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jan Blunck
     
  • This is the central patch of a cleanup series. In most cases there is no good
    reason why someone would want to use a dentry for itself. This series reflects
    that fact and embeds a struct path into nameidata.

    Together with the other patches of this series
    - it enforced the correct order of getting/releasing the reference count on
    pairs
    - it prepares the VFS for stacking support since it is essential to have a
    struct path in every place where the stack can be traversed
    - it reduces the overall code size:

    without patch series:
    text data bss dec hex filename
    5321639 858418 715768 6895825 6938d1 vmlinux

    with patch series:
    text data bss dec hex filename
    5320026 858418 715768 6894212 693284 vmlinux

    This patch:

    Switch from nd->{dentry,mnt} to nd->path.{dentry,mnt} everywhere.

    [akpm@linux-foundation.org: coding-style fixes]
    [akpm@linux-foundation.org: fix cifs]
    [akpm@linux-foundation.org: fix smack]
    Signed-off-by: Jan Blunck
    Signed-off-by: Andreas Gruenbacher
    Acked-by: Christoph Hellwig
    Cc: Al Viro
    Cc: Casey Schaufler
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Jan Blunck
     

08 Feb, 2008

1 commit


06 Feb, 2008

1 commit

  • The patch supports legacy (32-bit) capability userspace, and where possible
    translates 32-bit capabilities to/from userspace and the VFS to 64-bit
    kernel space capabilities. If a capability set cannot be compressed into
    32-bits for consumption by user space, the system call fails, with -ERANGE.

    FWIW libcap-2.00 supports this change (and earlier capability formats)

    http://www.kernel.org/pub/linux/libs/security/linux-privs/kernel-2.6/

    [akpm@linux-foundation.org: coding-syle fixes]
    [akpm@linux-foundation.org: use get_task_comm()]
    [ezk@cs.sunysb.edu: build fix]
    [akpm@linux-foundation.org: do not initialise statics to 0 or NULL]
    [akpm@linux-foundation.org: unused var]
    [serue@us.ibm.com: export __cap_ symbols]
    Signed-off-by: Andrew G. Morgan
    Cc: Stephen Smalley
    Acked-by: Serge Hallyn
    Cc: Chris Wright
    Cc: James Morris
    Cc: Casey Schaufler
    Signed-off-by: Erez Zadok
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Andrew Morgan
     

02 Feb, 2008

31 commits

  • Neil Brown points out that we're checking buf[size-1] in a couple places
    without first checking whether size is zero.

    Actually, given the implementation of simple_transaction_get(), buf[-1]
    is zero, so in both of these cases the subsequent check of the value of
    buf[size-1] will catch this case.

    But it seems fragile to depend on that, so add explicit checks for this
    case.

    Signed-off-by: J. Bruce Fields
    Acked-by: NeilBrown

    J. Bruce Fields
     
  • Neither EPERM and ENOENT map to valid errors for PUTROOTFH according to
    rfc 3530, and, if anything, ENOENT is likely to be slightly more
    informative; so don't bother mapping ENOENT to EPERM. (Probably this
    was originally done because one likely cause was that there is an fsid=0
    export but that it isn't permitted to this particular client. Now that
    we allow WRONGSEC returns, this is somewhat less likely.)

    In the long term we should work to make this situation less likely,
    perhaps by turning off nfsv4 service entirely in the absence of the
    pseudofs root, or constructing a pseudofilesystem root ourselves in the
    kernel as necessary.

    Thanks to Benny Halevy for pointing out this
    problem.

    Signed-off-by: J. Bruce Fields
    Cc: Benny Halevy

    J. Bruce Fields
     
  • Create a transport independent version of the svc_sock_names function.

    The toclose capability of the svc_sock_names service can be implemented
    using the svc_xprt_find and svc_xprt_close services.

    Signed-off-by: Tom Tucker
    Acked-by: Neil Brown
    Reviewed-by: Chuck Lever
    Reviewed-by: Greg Banks
    Signed-off-by: J. Bruce Fields

    Tom Tucker
     
  • Update the write handler for the portlist file to allow creating new
    listening endpoints on a transport. The general form of the string is:

    For example:

    echo "tcp 2049" > /proc/fs/nfsd/portlist

    This is intended to support the creation of a listening endpoint for
    RDMA transports without adding #ifdef code to the nfssvc.c file.

    Transports can also be removed as follows:

    '-'

    For example:

    echo "-tcp 2049" > /proc/fs/nfsd/portlist

    Attempting to add a listener with an invalid transport string results
    in EPROTONOSUPPORT and a perror string of "Protocol not supported".

    Attempting to remove an non-existent listener (.e.g. bad proto or port)
    results in ENOTCONN and a perror string of
    "Transport endpoint is not connected"

    Signed-off-by: Tom Tucker
    Acked-by: Neil Brown
    Reviewed-by: Chuck Lever
    Reviewed-by: Greg Banks
    Signed-off-by: J. Bruce Fields

    Tom Tucker
     
  • Move sk_list and sk_ready to svc_xprt. This involves close because these
    lists are walked by svcs when closing all their transports. So I combined
    the moving of these lists to svc_xprt with making close transport independent.

    The svc_force_sock_close has been changed to svc_close_all and takes a list
    as an argument. This removes some svc internals knowledge from the svcs.

    This code races with module removal and transport addition.

    Thanks to Simon Holm Thøgersen for a compile fix.

    Signed-off-by: Tom Tucker
    Acked-by: Neil Brown
    Reviewed-by: Chuck Lever
    Reviewed-by: Greg Banks
    Signed-off-by: J. Bruce Fields
    Cc: Simon Holm Thøgersen

    Tom Tucker
     
  • Modify the various kernel RPC svcs to use the svc_create_xprt service.

    Signed-off-by: Tom Tucker
    Acked-by: Neil Brown
    Reviewed-by: Chuck Lever
    Reviewed-by: Greg Banks
    Signed-off-by: J. Bruce Fields

    Tom Tucker
     
  • Document these checks a little better and inline, as suggested by Neil
    Brown (note both functions have two callers). Remove an obviously bogus
    check while we're there (checking whether unsigned value is negative).

    Signed-off-by: J. Bruce Fields
    Cc: Neil Brown

    J. Bruce Fields
     
  • The server silently ignores attempts to set the uid and gid on create.
    Based on the comment, this appears to have been done to prevent some
    overly-clever IRIX client from causing itself problems.

    Perhaps we should remove that hack completely. For now, at least, it
    makes sense to allow root (when no_root_squash is set) to set uid and
    gid.

    While we're there, since nfsd_create and nfsd_create_v3 share the same
    logic, pull that out into a separate function. And spell out the
    individual modifications of ia_valid instead of doing them both at once
    inside a conditional.

    Thanks to Roger Willcocks for the bug report
    and original patch on which this is based.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • This patch addresses a compatibility issue with a Linux NFS server and
    AIX NFS client.

    I have exported /export as fsid=0 with sec=krb5:krb5i
    I have mount --bind /home onto /export/home
    I have exported /export/home with sec=krb5i

    The AIX client mounts / -o sec=krb5:krb5i onto /mnt

    If I do an ls /mnt, the AIX client gets a permission error. Looking at
    the network traceIwe see a READDIR looking for attributes
    FATTR4_RDATTR_ERROR and FATTR4_MOUNTED_ON_FILEID. The response gives a
    NFS4ERR_WRONGSEC which the AIX client is not expecting.

    Since the AIX client is only asking for an attribute that is an
    attribute of the parent file system (pseudo root in my example), it
    seems reasonable that there should not be an error.

    In discussing this issue with Bruce Fields, I initially proposed
    ignoring the error in nfsd4_encode_dirent_fattr() if all that was being
    asked for was FATTR4_RDATTR_ERROR and FATTR4_MOUNTED_ON_FILEID, however,
    Bruce suggested that we avoid calling cross_mnt() if only these
    attributes are requested.

    The following patch implements bypassing cross_mnt() if only
    FATTR4_RDATTR_ERROR and FATTR4_MOUNTED_ON_FILEID are called. Since there
    is some complexity in the code in nfsd4_encode_fattr(), I didn't want to
    duplicate code (and introduce a maintenance nightmare), so I added a
    parameter to nfsd4_encode_fattr() that indicates whether it should
    ignore cross mounts and simply fill in the attribute using the passed in
    dentry as opposed to it's parent.

    Signed-off-by: Frank Filz
    Signed-off-by: J. Bruce Fields

    Frank Filz
     
  • The failure to return a stateowner from nfs4_preprocess_seqid_op() means
    in the case where a lock request is of a type incompatible with an open
    (due to, e.g., an application attempting a write lock on a file open for
    read), means that fs/nfsd/nfs4xdr.c:ENCODE_SEQID_OP_TAIL() never bumps
    the seqid as it should. The client, attempting to close the file
    afterwards, then gets an (incorrect) bad sequence id error. Worse, this
    prevents the open file from ever being closed, so we leak state.

    Thanks to Benny Halevy and Trond Myklebust for analysis, and to Steven
    Wilton for the report and extensive data-gathering.

    Cc: Benny Halevy
    Cc: Steven Wilton
    Cc: Trond Myklebust
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • When the callback channel fails, we inform the client of that by
    returning a cb_path_down error the next time it tries to renew its
    lease.

    If we wait most of a lease period before deciding that a callback has
    failed and that the callback channel is down, then we decrease the
    chances that the client will find out in time to do anything about it.

    So, mark the channel down as soon as we recognize that an rpc has
    failed. However, continue trying to recall delegations anyway, in hopes
    it will come back up. This will prevent more delegations from being
    given out, and ensure cb_path_down is returned to renew calls earlier,
    while still making the best effort to deliver recalls of existing
    delegations.

    Also fix a couple comments and remove a dprink that doesn't seem likely
    to be useful.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Fix various minor style violations.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Declare this variable in the one function where it's used, and clean up
    some minor style problems.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Fix bizarre indentation.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • We generate a unique cl_confirm for every new client; so if we've
    already checked that this cl_confirm agrees with the cl_confirm of
    unconf, then we already know that it does not agree with the cl_confirm
    of conf.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Again, the only way conf and unconf can have the same clientid is if
    they were created in the "probable callback update" case of setclientid,
    in which case we already know that the cl_verifier fields must agree.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • If conf and unconf are both found in the lookup by cl_clientid, then
    they share the same cl_clientid. We always create a unique new
    cl_clientid field when creating a new client--the only exception is the
    "probable callback update" case in setclientid, where we copy the old
    cl_clientid from another clientid with the same name.

    Therefore two clients with the same cl_client field also always share
    the same cl_name field, and a couple of the checks here are redundant.

    Thanks to Simon Holm Thøgersen for a compile fix.

    Signed-off-by: J. Bruce Fields
    Cc: Simon Holm Thøgersen

    J. Bruce Fields
     
  • Using a counter instead of the nanoseconds value seems more likely to
    produce a unique cl_confirm.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • We're supposed to generate a different cl_confirm verifier for each new
    client, so these to cl_confirm values should never be the same.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Most of these comments just summarize the code.

    The matching of code to the cases described in the RFC may still be
    useful, though; add specific section references to make that easier to
    follow. Also update references to the outdated RFC 3010.

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • While we're here, let's remove the redundant (and now wrong) pathname in
    the comment, and the #ifdef __KERNEL__'s.

    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • This header is used only in a few places in fs/nfsd, so there seems to
    be little point to having it in include/. (Thanks to Robert Day for
    pointing this out.)

    Cc: Robert P. J. Day
    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Newer server features such as nfsv4 and gss depend on proc to work, so a
    failure to initialize the proc files they need should be treated as
    fatal.

    Thanks to Andrew Morton for style fix and compile fix in case where
    CONFIG_NFSD_V4 is undefined.

    Cc: Andrew Morton
    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • I assume the reason failure of creation was ignored here was just to
    continue support embedded systems that want nfsd but not proc.

    However, in cases where proc is supported it would be clearer to fail
    entirely than to come up with some features disabled.

    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • There's really nothing much the caller can do if cache unregistration
    fails. And indeed, all any caller does in this case is print an error
    and continue. So just return void and move the printk's inside
    cache_unregister.

    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • If the reply cache initialization fails due to a kmalloc failure,
    currently we try to soldier on with a reduced (or nonexistant) reply
    cache.

    Better to just fail immediately: the failure is then much easier to
    understand and debug, and it could save us complexity in some later
    code. (But actually, it doesn't help currently because the cache is
    also turned off in some odd failure cases; we should probably find a
    better way to handle those failure cases some day.)

    Fix some minor style problems while we're at it, and rename
    nfsd_cache_init() to remove the need for a comment describing it.

    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Handle the failure case here with something closer to the standard
    kernel style.

    Doesn't really matter for now, but I'd like to add a few more failure
    cases, and then this'll help.

    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • We forgot to shut down the nfs4 state and idmapping code in this case.

    Acked-by: NeilBrown
    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • The length "nbytes" passed into read_buf should never be negative, but
    we check only for too-large values of "nbytes", not for too-small
    values. Make nbytes unsigned, so it's clear that the former tests are
    sufficient. (Despite this read_buf() currently correctly returns an xdr
    error in the case of a negative length, thanks to an unsigned
    comparison with size_of() and bounds-checking in kmalloc(). This seems
    very fragile, though.)

    Signed-off-by: J. Bruce Fields

    J. Bruce Fields
     
  • Signed-off-by: Chuck Lever
    Acked-By: NeilBrown
    Signed-off-by: J. Bruce Fields

    Chuck Lever
     
  • Clean up: path name lengths are unsigned on the wire, negative lengths
    are not meaningful natively either.

    Signed-off-by: Chuck Lever
    Acked-By: NeilBrown
    Signed-off-by: J. Bruce Fields

    Chuck Lever