28 May, 2016
1 commit
-
Most users of IS_ERR_VALUE() in the kernel are wrong, as they
pass an 'int' into a function that takes an 'unsigned long'
argument. This happens to work because the type is sign-extended
on 64-bit architectures before it gets converted into an
unsigned type.However, anything that passes an 'unsigned short' or 'unsigned int'
argument into IS_ERR_VALUE() is guaranteed to be broken, as are
8-bit integers and types that are wider than 'unsigned long'.Andrzej Hajda has already fixed a lot of the worst abusers that
were causing actual bugs, but it would be nice to prevent any
users that are not passing 'unsigned long' arguments.This patch changes all users of IS_ERR_VALUE() that I could find
on 32-bit ARM randconfig builds and x86 allmodconfig. For the
moment, this doesn't change the definition of IS_ERR_VALUE()
because there are probably still architecture specific users
elsewhere.Almost all the warnings I got are for files that are better off
using 'if (err)' or 'if (err < 0)'.
The only legitimate user I could find that we get a warning for
is the (32-bit only) freescale fman driver, so I did not remove
the IS_ERR_VALUE() there but changed the type to 'unsigned long'.
For 9pfs, I just worked around one user whose calling conventions
are so obscure that I did not dare change the behavior.I was using this definition for testing:
#define IS_ERR_VALUE(x) ((unsigned long*)NULL == (typeof (x)*)NULL && \
unlikely((unsigned long long)(x) >= (unsigned long long)(typeof(x))-MAX_ERRNO))which ends up making all 16-bit or wider types work correctly with
the most plausible interpretation of what IS_ERR_VALUE() was supposed
to return according to its users, but also causes a compile-time
warning for any users that do not pass an 'unsigned long' argument.I suggested this approach earlier this year, but back then we ended
up deciding to just fix the users that are obviously broken. After
the initial warning that caused me to get involved in the discussion
(fs/gfs2/dir.c) showed up again in the mainline kernel, Linus
asked me to send the whole thing again.[ Updated the 9p parts as per Al Viro - Linus ]
Signed-off-by: Arnd Bergmann
Cc: Andrzej Hajda
Cc: Andrew Morton
Link: https://lkml.org/lkml/2016/1/7/363
Link: https://lkml.org/lkml/2016/5/27/486
Acked-by: Srinivas Kandagatla # For nvmem part
Signed-off-by: Linus Torvalds
23 Aug, 2015
1 commit
-
Some use of those functions were providing unitialized values to those
functions. Notably, when reading 0 bytes from an empty file on a 9P
filesystem, the return code of read() was not 0.Tested with this simple program:
#include
#include
#include
#include
#includeint main(int argc, const char **argv)
{
assert(argc == 2);
char buffer[256];
int fd = open(argv[1], O_RDONLY|O_NOCTTY);
assert(fd >= 0);
assert(read(fd, buffer, 0) == 0);
return 0;
}Cc: stable@vger.kernel.org # v4.1
Signed-off-by: Vincent Bernat
Signed-off-by: Al Viro
05 Jul, 2015
3 commits
-
if server claims to have written/read more than we'd told it to,
warn and cap the claimed byte count to avoid advancing more than
we are ready to. -
Braino in "9p: switch p9_client_write() to passing it struct iov_iter *";
if response is impossible to parse and we discard the request, get the
out of the loop right there.Cc: stable@vger.kernel.org
Signed-off-by: Al Viro -
If we'd already sent a request and decide to abort it, we *must*
issue TFLUSH properly and not just blindly reuse the tag, or
we'll get seriously screwed when response eventually arrives
and we confuse it for response to later request that had reused
the same tag.Cc: stable@vger.kernel.org # v3.2 and later
Signed-off-by: Al Viro
12 Apr, 2015
4 commits
-
it's almost always equal to current_fsuid(), but there's an exception -
if the first writeback fid is opened by non-root *and* that happens before
root has done any lookups in /, we end up doing attach for root. The
current code leaves the resulting FID owned by root from the server POV
and by non-root from the client one. Unfortunately, it means that e.g.
massive dcache eviction will leave that user buggered - they'll end
up redoing walks from / *and* picking that FID every time. As soon as
they try to create something, the things will get nasty.Signed-off-by: Al Viro
-
... and make it loop
Signed-off-by: Al Viro
-
... and make it loop until it's done
Signed-off-by: Al Viro
-
... and have get_user_pages_fast() mapping fewer pages than requested
to generate a short read/write.Signed-off-by: Al Viro
16 Jul, 2014
1 commit
-
Signed-off-by: Fabian Frederick
Signed-off-by: David S. Miller
26 Mar, 2014
4 commits
-
This request state is mostly useless, and properly implementing it
for RDMA would require an extra lock to be taken in handle_recv()
and in rdma_cancel() to avoid this race:handle_recv() rdma_cancel()
. .
. if req->state == SENT
req->state = RCVD .
. req->state = FLSHSo just get rid of it.
Signed-off-by: Simon Derr
Signed-off-by: Eric Van Hensbergen -
And move transport-specific code out of net/9p/client.c
Signed-off-by: Simon Derr
Signed-off-by: Eric Van Hensbergen -
Mark function as static in net/9p/client.c because it is not used
outside this file.This eliminates the following warning in net/9p/client.c:
net/9p/client.c:207:18: warning: no previous prototype for ‘p9_fcall_alloc’ [-Wmissing-prototypes]Signed-off-by: Rashika Kheria
Reviewed-by: Josh Triplett
Signed-off-by: Eric Van Hensbergen -
We need barriers to guarantee this pattern works as intended:
[w] req->rc, 1 [r] req->status, 1
wmb rmb
[w] req->status, 1 [r] req->rcWhere the wmb ensures that rc gets written before status,
and the rmb ensures that if you observe status == 1, rc is the new value.Signed-off-by: Dominique Martinet
Signed-off-by: Eric Van Hensbergen
24 Nov, 2013
1 commit
-
A few releases back a patch made virtio the default transport, however
it was done in a way which side-stepped the mechanism put in place to
allow for this selection. This patch cleans that up while maintaining
virtio as the default transport.Signed-off-by: Eric Van Hensbergen
12 Sep, 2013
1 commit
-
Pull 9p updates from Eric Van Hensbergen:
"Minor 9p fixes and tweaks for 3.12 merge windowThe first fixes namespace issues which causes a kernel NULL pointer
dereference, the second fixes uevent handling to work better with
udev, and the third switches some code to use srlcpy instead of
strncpy in order to be safer.All changes have been baking in for-next for at least 2 weeks"
* tag 'for-linus-3.12-merge' of git://git.kernel.org/pub/scm/linux/kernel/git/ericvh/v9fs:
fs/9p: avoid accessing utsname after namespace has been torn down
9p: send uevent after adding/removing mount_tag attribute
fs: 9p: use strlcpy instead of strncpy
26 Aug, 2013
1 commit
-
During trinity fuzzing in a kvmtool guest, I stumbled across the
following:Unable to handle kernel NULL pointer dereference at virtual address 00000004
PC is at v9fs_file_do_lock+0xc8/0x1a0
LR is at v9fs_file_do_lock+0x48/0x1a0
[] (v9fs_file_do_lock+0xc8/0x1a0) from [] (locks_remove_flock+0x8c/0x124)
[] (locks_remove_flock+0x8c/0x124) from [] (__fput+0x58/0x1e4)
[] (__fput+0x58/0x1e4) from [] (task_work_run+0xac/0xe8)
[] (task_work_run+0xac/0xe8) from [] (do_exit+0x6bc/0x8d8)
[] (do_exit+0x6bc/0x8d8) from [] (do_group_exit+0x3c/0xb0)
[] (do_group_exit+0x3c/0xb0) from [] (__wake_up_parent+0x0/0x18)I believe this is due to an attempt to access utsname()->nodename, after
exit_task_namespaces() has been called, leaving current->nsproxy->uts_ns
as NULL and causing the above dereference.A similar issue was fixed for lockd in 9a1b6bf818e7 ("LOCKD: Don't call
utsname()->nodename from nlmclnt_setlockargs"), so this patch attempts
something similar for 9pfs.Cc: Eric Van Hensbergen
Cc: Ron Minnich
Cc: Latchesar Ionkov
Cc: Trond Myklebust
Signed-off-by: Will Deacon
Signed-off-by: Eric Van Hensbergen
31 Jul, 2013
1 commit
-
This patch reverts commit
80b45261a0b263536b043c5ccfc4ba4fc27c2acc
which was implementing a 'cancelled' functionality to notify that
a cancelled request will not be replied.This implementation was not used anywhere and therefore removed.
Signed-off-by: Andi Shyti
Signed-off-by: David S. Miller
08 Jul, 2013
4 commits
-
RDMA needs to post a buffer for each incoming reply.
Hence it needs to keep count of these and needs to be
aware of whether a flushed request has received a reply
or not.This patch adds the cancelled() callback to the transport modules.
It is called when RFLUSH has been received and that the corresponding
request will never receive a reply.Signed-off-by: Simon Derr
Signed-off-by: Eric Van Hensbergen -
In rdma_request():
If an error occurs between posting the recv and the send,
there will be a reply context posted without a pending
request.
Since there is no way to "un-post" it, we remember it and
skip post_recv() for the next request.Signed-off-by: Simon Derr
Signed-off-by: Eric Van Hensbergen -
Signed-off-by: Simon Derr
Signed-off-by: Eric Van Hensbergen -
The current code assumes that when a request in the request array
does have a tc, it also has a rc.This is normally true, but not always : when using RDMA, req->rc
will temporarily be set to NULL after the request has been sent.
That is usually OK though, as when the reply arrives, req->rc will be
reassigned to a sane value before the request is recycled.But there is a catch : if the request is flushed, the reply will never
arrive, and req->rc will be NULL, but not req->tc.This patch fixes p9_tag_alloc to take this into account.
Signed-off-by: Simon Derr
Signed-off-by: Eric Van Hensbergen
28 May, 2013
3 commits
-
For zero copy request, error will be encoded in the user space buffer.
So copy the error code correctly using copy_from_user. Here we use the
extra bytes we allocate for zero copy request. If total error details
are more than P9_ZC_HDR_SZ - 7 bytes, we return -EFAULT. The patch also
avoid a memory allocation in the error path.Signed-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen -
Make the default 9p experience better by defaulting to virtio transport if present.
These days most of the users are using 9p in a virtualized setupSigned-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen -
If we dont' specify a protocol version default to 9P2000.L. 9P2000.L
have better support for posix semantic and is where all the recent development
is happening.Signed-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen
12 Feb, 2013
2 commits
-
9p has thre strucrtures that can encode inode stat information. Modify
all of those structures to contain kuid_t and kgid_t values. Modify
he wire encoders and decoders of those structures to use 'u' and 'g' instead of
'd' in the format string where uids and gids are present.This results in all kuid and kgid conversion to and from on the wire values
being performed by the same code in protocol.c where the client is known
at the time of the conversion.Cc: Eric Van Hensbergen
Cc: Ron Minnich
Cc: Latchesar Ionkov
Signed-off-by: Eric W. Biederman -
Modify the p9_client_rpc format specifiers of every function that
directly transmits a uid or a gid from 'd' to 'u' or 'g' as
appropriate.Modify those same functions to take kuid_t and kgid_t parameters
instead of uid_t and gid_t parameters.Cc: Eric Van Hensbergen
Cc: Ron Minnich
Cc: Latchesar Ionkov
Signed-off-by: Eric W. Biederman
07 Sep, 2012
1 commit
-
While working on a modified server I had the Linux clients crash
a few times. This lead me to find this:Some error codes are directly extracted from the server replies.
A malformed server reply could contain an invalid error code, with a
very large value. If this value is then passed to ERR_PTR() it will
not be properly detected as an error code by IS_ERR() and as a result
the kernel will dereference an invalid pointer.This patch tries to avoid this.
Signed-off-by: Simon Derr
Signed-off-by: Eric Van Hensbergen
05 Jun, 2012
1 commit
-
A recent commit that removed unnecessary casts of pointers
to the same type uncovered a missing __force cast.Add it.
Reported by: Ben Hutchings
Signed-off-by: Joe Perches
Signed-off-by: David S. Miller
04 Jun, 2012
1 commit
-
Adding casts of objects to the same type is unnecessary
and confusing for a human reader.For example, this cast:
int y;
int *p = (int *)&y;I used the coccinelle script below to find and remove these
unnecessary casts. I manually removed the conversions this
script produces of casts with __force and __user.@@
type T;
T *p;
@@- (T *)p
+ pSigned-off-by: Joe Perches
Signed-off-by: David S. Miller
16 Apr, 2012
1 commit
-
Use of "unsigned int" is preferred to bare "unsigned" in net tree.
Signed-off-by: Eric Dumazet
Signed-off-by: David S. Miller
27 Feb, 2012
2 commits
-
When a Tclunk or Tremove request is flushed, the fid is not freed on the
server.p9_client_clunk() should retry once on interrupt, then if interrupted
again, leak the fid for the duration of the connection.p9_client_remove() should call p9_client_clunk() on interrupt
instead of unconditionally destroying the fid.Signed-off-by: Jim Garlick
Signed-off-by: Eric Van Hensbergen -
When a signal is received while sending a Tflush, the client,
which has recursed into p9_client_rpc() while sending another request,
should wait for Rflush as long as the transport is still up.Signed-off-by: Jim Garlick
Signed-off-by: Eric Van Hensbergen
06 Jan, 2012
1 commit
-
Reduce object size by deduplicating formats.
Use vsprintf extension %pV.
Rename P9_DPRINTK uses to p9_debug, align arguments.
Add function for _p9_debug and macro to add __func__.
Add missing "\n"s to p9_debug uses.
Remove embedded function names as p9_debug adds it.
Remove P9_EPRINTK macro and convert use to pr_.
Add and use pr_fmt and pr_.$ size fs/9p/built-in.o*
text data bss dec hex filename
62133 984 16000 79117 1350d fs/9p/built-in.o.new
67342 984 16928 85254 14d06 fs/9p/built-in.o.old
$ size net/9p/built-in.o*
text data bss dec hex filename
88792 4148 22024 114964 1c114 net/9p/built-in.o.new
94072 4148 23232 121452 1da6c net/9p/built-in.o.oldSigned-off-by: Joe Perches
Signed-off-by: Eric Van Hensbergen
25 Oct, 2011
5 commits
-
This helps in more control over debugging.
root@qemu-img-64:~# ls /pass/123
ls: cannot access /pass/123: No such file or directory
root@qemu-img-64:~# cat /sys/kernel/debug/tracing/trace
# tracer: nop
#
# TASK-PID CPU# TIMESTAMP FUNCTION
# | | | | |
ls-1536 [001] 70.928584: 9p_protocol_dump: clnt 18446612132784021504 P9_TWALK(tag = 1)
000: 16 00 00 00 6e 01 00 01 00 00 00 02 00 00 00 01
010: 00 03 00 31 32 33 00 00 00 ff ff ff ff 00 00 00ls-1536 [001] 70.928587:
=> trace_9p_protocol_dump
=> p9pdu_finalize
=> p9_client_rpc
=> p9_client_walk
=> v9fs_vfs_lookup
=> d_alloc_and_lookup
=> walk_component
=> path_lookupat
ls-1536 [000] 70.929696: 9p_protocol_dump: clnt 18446612132784021504 P9_RLERROR(tag = 1)
000: 0b 00 00 00 07 01 00 02 00 00 00 4e 03 00 02 00
010: 00 00 00 00 03 00 02 00 00 00 00 00 ff 43 00 00ls-1536 [000] 70.929697:
=> trace_9p_protocol_dump
=> p9_client_rpc
=> p9_client_walk
=> v9fs_vfs_lookup
=> d_alloc_and_lookup
=> walk_component
=> path_lookupat
=> do_path_lookupSigned-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen -
Without this msize=4294967295 will result in a crash
Signed-off-by: Dan Carpenter
Signed-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen -
Instead of saying all integer argument option should be listed in the beginning
move integer parsing to each option type.Signed-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen -
We dereferenced "req->tc" and "req->rc" before checking for NULL.
Signed-off-by: Dan Carpenter
Signed-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen -
* remove lot of update to different data structure
* add a seperate callback for zero copy request.
* above makes non zero copy code path simpler
* remove conditionalizing TREAD/TREADDIR/TWRITE in the zero copy path
* Fix the dotu p9_check_errors with zero copy. Add sufficient doc around
* Add support for both in and output buffers in zero copy callback
* pin and unpin pages in the same context
* use helpers instead of defining page offset and rest of page ourself
* Fix mem leak in p9_check_errors
* Remove 'E' and 'F' in p9pdu_vwritefSigned-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen
23 Jul, 2011
1 commit
-
msize represents the maximum PDU size that includes P9_IOHDRSZ.
Signed-off-by: Venkateswararao Jujjuri "
Signed-off-by: Aneesh Kumar K.V
Signed-off-by: Eric Van Hensbergen
