Do the initialization in the caller, and clarify that the only failure
ever possible here was due to allocation.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
It'll be useful to have connection allocation and initialization as
separate functions.
Also, note we'd been ignoring the alloc_conn error return in
bind_conn_to_session.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Something like creating a client with setclientid and then trying to
confirm it with create_session may not crash the server, but I'm not
completely positive of that, and in any case it's obviously bad client
behavior.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
I added cr_flavor to the data compared in same_creds without any
justification, in d5497fc693 "nfsd4: move
rq_flavor into svc_cred".
Recent client changes then started making
mount -osec=krb5 server:/export /mnt/
echo "hello" >/mnt/TMP
umount /mnt/
mount -osec=krb5i server:/export /mnt/
echo "hello" >/mnt/TMP
to fail due to a clid_inuse on the second open.
Mounting sequentially like this with different flavors probably isn't
that common outside artificial tests. Also, the real bug here may be
that the server isn't just destroying the former clientid in this case
(because it isn't good enough at recognizing when the old state is
gone). But it prompted some discussion and a look back at the spec, and
I think the check was probably wrong. Fix and document.
Cc: stable@kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
The 'buf' is prepared with null termination with intention of using it for
this purpose, but 'name' is passed instead!
Signed-off-by: Malahal Naineni <malahal@us.ibm.com>
Cc: stable@vger.kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Change the call to PTR_ERR to access the value just tested by IS_ERR.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression e,e1;
@@
(
if (IS_ERR(e)) { ... PTR_ERR(e) ... }
|
if (IS_ERR(e=e1)) { ... PTR_ERR(e) ... }
|
*if (IS_ERR(e))
{ ...
* PTR_ERR(e1)
... }
)
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
You can use nfsd/portlist to give nfsd additional sockets to listen on.
In theory you can also remove listening sockets this way. But nobody's
ever done that as far as I can tell.
Also this was partially broken in 2.6.25, by
a217813f90 "knfsd: Support adding
transports by writing portlist file".
(Note that we decide whether to take the "delfd" case by checking for a
digit--but what's actually expected in that case is something made by
svc_one_sock_name(), which won't begin with a digit.)
So, let's just rip out this stuff.
Acked-by: NeilBrown <neilb@suse.de>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Processes that open and close multiple files may end up setting this
oo_last_closed_stid without freeing what was previously pointed to.
This can result in a major leak, visible for example by watching the
nfsd4_stateids line of /proc/slabinfo.
Reported-by: Cyril B. <cbay@excellency.fr>
Tested-by: Cyril B. <cbay@excellency.fr>
Cc: stable@vger.kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
These are only needed by nfs-utils. But I needed to remind myself how
they worked recently and thought this might be helpful. It's short and
incomplete for now as I was only interested in startup, shutdown, and
configuration of listening sockets.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Matter of taste, I suppose, but svc_recv breaks up naturally into:
allocate pages and setup arg
dequeue (wait for, if necessary) next socket
do something with that socket
And I find it easier to read when it doesn't go on for pages and pages.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Note this isn't used outside svc_xprt.c.
May as well move it so we don't need a declaration while we're here.
Also remove an outdated comment.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
svc_recv() returns only -EINTR or -EAGAIN. If we really want to worry
about the case where it has a bug that causes it to return something
else, we could stick a WARN() in svc_recv. But it's silly to require
every caller to have all this boilerplate to handle that case.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
The only errors returned from xpo_recvfrom have been -EAGAIN and
-EAFNOSUPPORT. The latter was removed by a previous patch. That leaves
only -EAGAIN, which is treated just like 0 by the caller (svc_recv).
So, just ditch -EAGAIN and return 0 instead.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
None of the callers should see an unsupported address family (only one
of them even bothers to check for that case), so just check for the
buggy case in svc_addr_len and don't bother elsewhere.
Acked-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
"port" in all these functions is always NFS_PORT.
nfsd can already be run on a nonstandard port using the "nfsd/portlist"
interface.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Whenever we clear XPT_BUSY we should call svc_xprt_enqueue(). Without
that we may fail to notice any events (such as new connections) that
arrived while XPT_BUSY was set.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Mainly, use the kernel standard
err = -ERROR;
if (something_bad)
goto out;
normal case;
rather than
if (something_bad)
err = -ERROR
else {
normal case;
}
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Server threads are not running at this point, but svc_age_temp_xprts
still may be, so we need this locking.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
struct file_lock is pretty large and really ought not live on the stack.
On my x86_64 machine, they're almost 200 bytes each.
(gdb) p sizeof(struct file_lock)
$1 = 192
...allocate them dynamically instead.
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
The code checks for a NULL filp and handles it gracefully just before
this BUG_ON.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
stateid_setter should be matched to op_set_currentstateid, rather than
op_get_currentstateid.
Signed-off-by: Bryan Schumaker <bjschuma@netapp.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
locks.c doesn't use the BKL anymore and there is no fi_perfile field.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
The rules for fl_type are rather convoluted. Typically it's treated as
holding specific values, except in the case of LOCK_MAND, in which case
it can be or'ed with LOCK_READ|LOCK_WRITE.
On some arches F_WRLCK == 2 and F_UNLCK == 3, so and'ing with F_WRLCK will also
catch the F_UNLCK case. It's unlikely in either case here that we'd ever see
F_UNLCK since those shouldn't end up on any lists, but it's still best to be
consistent.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
The rpc server tries to ensure that there will be room to send a reply
before it receives a request.
It does this by tracking, in xpt_reserved, an upper bound on the total
size of the replies that is has already committed to for the socket.
Currently it is adding in the estimate for a new reply *before* it
checks whether there is space available. If it finds that there is not
space, it then subtracts the estimate back out.
This may lead the subsequent svc_xprt_enqueue to decide that there is
space after all.
The results is a svc_recv() that will repeatedly return -EAGAIN, causing
server threads to loop without doing any actual work.
Cc: stable@vger.kernel.org
Reported-by: Michael Tokarev <mjt@tls.msk.ru>
Tested-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
svc_tcp_sendto sets XPT_CLOSE if we fail to transmit the entire reply.
However, the XPT_CLOSE won't be acted on immediately. Meanwhile other
threads could send further replies before the socket is really shut
down. This can manifest as data corruption: for example, if a truncated
read reply is followed by another rpc reply, that second reply will look
to the client like further read data.
Symptoms were data corruption preceded by svc_tcp_sendto logging
something like
kernel: rpc-srv/tcp: nfsd: sent only 963696 when sending 1048708 bytes - shutting down socket
Cc: stable@vger.kernel.org
Reported-by: Malahal Naineni <malahal@us.ibm.com>
Tested-by: Malahal Naineni <malahal@us.ibm.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Examination of svc_tcp_clear_pages shows that it assumes sk_tcplen is
consistent with sk_pages[] (in particular, sk_pages[n] can't be NULL if
sk_tcplen would lead us to expect n pages of data).
svc_tcp_restore_pages zeroes out sk_pages[] while leaving sk_tcplen.
This is OK, since both functions are serialized by XPT_BUSY. However,
that means the inconsistency must be repaired before dropping XPT_BUSY.
Therefore we should be ensuring that svc_tcp_save_pages repairs the
problem before exiting svc_tcp_recv_record on error.
Symptoms were a BUG() in svc_tcp_clear_pages.
Cc: stable@vger.kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Commit d5497fc693 "nfsd4: move rq_flavor
into svc_cred" forgot to remove cl_flavor from the client, leaving two
places (cl_flavor and cl_cred.cr_flavor) for the flavor to be stored.
After that patch, the latter was the one that was updated, but the
former was the one that the callback used.
Symptoms were a long delay on utime(). This is because the utime()
generated a setattr which recalled a delegation, but the cb_recall was
ignored by the client because it had the wrong security flavor.
Cc: stable@vger.kernel.org
Tested-by: Jamie Heilman <jamie@audible.transient.net>
Reported-by: Jamie Heilman <jamie@audible.transient.net>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Following a report of a crash during an automount expire I found that
the locking in fs/autofs4/expire.c:get_next_positive_subdir() was wrong.
Not only is the locking wrong but the function is more complex than it
needs to be.
The function is meant to calculate (and dget) the next entry in the list
of directories contained in the root of an autofs mount point (an autofs
indirect mount to be precise). The main problem was that the d_lock of
the owner of the list was not being taken when walking the list, which
lead to list corruption under load. The only other lock that needs to
be taken is against the next dentry candidate so it can be checked for
usability.
Signed-off-by: Ian Kent <raven@themaw.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
iQIcBAABAgAGBQJQIVvKAAoJECObm247sIsiSv0P/3vTTHqBHhhw9ZAcO+K6xyjo
kccytHuMko1+H+J8G2ovvC1YMZzPF/bp81yTDv+H5xjYEHaE5iDRz91X4x+0oP/z
qAYYdOfufWAXNJqlHL6xNtYKMTnK0jrtqTYkkeKRZbbdyEkvEa0cLbJpHWB16IDW
jECk1sTmVlCifYiN9cgV8GyNOenXM7/fEtnpsbB1kzmAtBW7sXCEzA2KiEuvtL7S
Nnjzchu3lvXkGg26qF9zv5qAj4nMOY8uDsQNNJtCxfbWjdG2W5PylqHjeUbE/g95
C3Kxrw9QZsM8zY/9KO/gGxpQ0S9zMiX9999ySFJtIdy0vcTxlXI5kzJRgiJRlw/y
KhiKrD3lr0Vlf4vb4YaDAeN07VEsXOthH5kkgswK7rfXwAFhhjub/ezn8xePR8/2
tbA9C+koMQTc5AbifbNhEGz+Qzjc5HqislM5zeGAL8wHTsgi9yQYjH1AWljM3dAU
ST/Gqe6rHIXpgrL1YSY0MTpVwaOXpokILVcOcnPpY3HcgatrQMRhNSqdxGMWygmA
NUbfL79kHYVmlkaPIsEymh9tx0wuDe0KyC/hzxm/dWB4MRGzE9AhMc6Hj4N5gXQY
H8qbMA5nPV4kS1xZNUc7wRK9YxRbCi7NKVoQba89yR3PBWwceSnGdKMA57G82gOm
uwe5wfAkno1KCk/m3zXk
=fUJz
-----END PGP SIGNATURE-----
Merge tag 'vfio-for-v3.6-rc1' of git://github.com/awilliam/linux-vfio
Pull VFIO fix from Alex Williamson:
"Just a trivial patch to include vfio.h in the installed headers so we
can complete userspace integration into QEMU."
* tag 'vfio-for-v3.6-rc1' of git://github.com/awilliam/linux-vfio:
vfio: Include vfio.h in installed headers
* On machines with large MMIO/PCI E820 spaces we fail to boot b/c
we failed to pre-allocate large enough virtual space for extend_brk.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
iQEcBAABAgAGBQJQKlV9AAoJEFjIrFwIi8fJZh4H/0ZlRrgG+8mqwCM+pcyYY+2a
zqnOrfYUO/aO26oqiOQUrn4quLAElhBuJK19uSj8fckMMZ+sr5rTJTaXmT6b7F7N
pgTXsKQCYAJ2NNGHVSQ73KYjOUeEW3woDSQZo0y/GRzOjiQsxpoFc8PS94ZieUNT
G6a8ECZBRv3fz8nAuJlhGV/suqHGOLJ0pwum1gHGOzaH3ZoZVtaQv5LhGYctJspU
yF5bdeD0qjCbseVtJ72tyxzLxMwLpJtdy2MbSwIv5JGuszj0nRmL4oa7Vc4vYdyv
p+FrNmbDAZ1j61z1PhBZPmgzwba2LTXtIWhR2zsGJgqlJNzMUtlNkff1kT3NeE0=
=Gl6V
-----END PGP SIGNATURE-----
Merge tag 'stable/for-linus-3.6-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen
Pull Xen fix from Konrad Rzeszutek Wilk:
"Way back in v3.5 we added a mechanism to populate back pages that were
released (they overlapped with MMIO regions), but neglected to reserve
the proper amount of virtual space for extend_brk to work properly.
Coincidentally some other commit aligned the _brk space to larger area
so I didn't trigger this until it was run on a machine with more than
2GB of MMIO space."
* On machines with large MMIO/PCI E820 spaces we fail to boot b/c
we failed to pre-allocate large enough virtual space for extend_brk.
* tag 'stable/for-linus-3.6-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen:
xen/p2m: Reserve 8MB of _brk space for P2M leafs when populating back.
