kernel/linux.git/net/9p/client.c, branch v6.12.80

9p/net: fix improper handling of bogus negative read/write replies

2025-05-02T05:59:20+00:00

[ Upstream commit d0259a856afca31d699b706ed5e2adf11086c73b ] In p9_client_write() and p9_client_read_once(), if the server incorrectly replies with success but a negative write/read count then we would consider written (negative) <= rsize (positive) because both variables were signed. Make variables unsigned to avoid this problem. The reproducer linked below now fails with the following error instead of a null pointer deref: 9pnet: bogus RWRITE count (4294967295 > 3) Reported-by: Robert Morris Closes: https://lore.kernel.org/16271.1734448631@26-5-164.dynamic.csail.mit.edu Message-ID: <20250319-9p_unsigned_rw-v3-1-71327f1503d0@codewreck.org> Reviewed-by: Christian Schoenebeck Signed-off-by: Dominique Martinet Signed-off-by: Sasha Levin

9p: fix slab cache name creation for real

2024-10-21T22:41:29+00:00

This was attempted by using the dev_name in the slab cache name, but as Omar Sandoval pointed out, that can be an arbitrary string, eg something like "/dev/root". Which in turn trips verify_dirent_name(), which fails if a filename contains a slash. So just make it use a sequence counter, and make it an atomic_t to avoid any possible races or locking issues. Reported-and-tested-by: Omar Sandoval Link: https://lore.kernel.org/all/ZxafcO8KWMlXaeWE@telecaster.dhcp.thefacebook.com/ Fixes: 79efebae4afc ("9p: Avoid creating multiple slab caches with the same name") Acked-by: Vlastimil Babka Cc: Dominique Martinet Cc: Thorsten Leemhuis Signed-off-by: Linus Torvalds

9p: Avoid creating multiple slab caches with the same name

2024-09-22T20:51:27+00:00

In the spirit of [1], avoid creating multiple slab caches with the same name. Instead, add the dev_name into the mix. [1]: https://lore.kernel.org/all/20240807090746.2146479-1-pedro.falcato@gmail.com/ Signed-off-by: Pedro Falcato Reported-by: syzbot+3c5d43e97993e1fa612b@syzkaller.appspotmail.com Message-ID: <20240807094725.2193423-1-pedro.falcato@gmail.com> Signed-off-by: Dominique Martinet

Merge tag '9p-for-6.10-rc2' of https://github.com/martinetd/linux

2024-05-29T16:25:15+00:00

Pull 9p fixes from Dominique Martinet: "Two fixes headed to stable trees: - a trace event was dumping uninitialized values - a missing lock that was thought to have exclusive access, and it turned out not to" * tag '9p-for-6.10-rc2' of https://github.com/martinetd/linux: 9p: add missing locking around taking dentry fid list net/9p: fix uninit-value in p9_client_rpc()

net/9p: fix uninit-value in p9_client_rpc()

2024-05-21T12:27:28+00:00

Syzbot with the help of KMSAN reported the following error: BUG: KMSAN: uninit-value in trace_9p_client_res include/trace/events/9p.h:146 [inline] BUG: KMSAN: uninit-value in p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 trace_9p_client_res include/trace/events/9p.h:146 [inline] p9_client_rpc+0x1314/0x1340 net/9p/client.c:754 p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 legacy_get_tree+0x114/0x290 fs/fs_context.c:662 vfs_get_tree+0xa7/0x570 fs/super.c:1797 do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 path_mount+0x742/0x1f20 fs/namespace.c:3679 do_mount fs/namespace.c:3692 [inline] __do_sys_mount fs/namespace.c:3898 [inline] __se_sys_mount+0x725/0x810 fs/namespace.c:3875 __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 do_syscall_64+0xd5/0x1f0 entry_SYSCALL_64_after_hwframe+0x6d/0x75 Uninit was created at: __alloc_pages+0x9d6/0xe70 mm/page_alloc.c:4598 __alloc_pages_node include/linux/gfp.h:238 [inline] alloc_pages_node include/linux/gfp.h:261 [inline] alloc_slab_page mm/slub.c:2175 [inline] allocate_slab mm/slub.c:2338 [inline] new_slab+0x2de/0x1400 mm/slub.c:2391 ___slab_alloc+0x1184/0x33d0 mm/slub.c:3525 __slab_alloc mm/slub.c:3610 [inline] __slab_alloc_node mm/slub.c:3663 [inline] slab_alloc_node mm/slub.c:3835 [inline] kmem_cache_alloc+0x6d3/0xbe0 mm/slub.c:3852 p9_tag_alloc net/9p/client.c:278 [inline] p9_client_prepare_req+0x20a/0x1770 net/9p/client.c:641 p9_client_rpc+0x27e/0x1340 net/9p/client.c:688 p9_client_create+0x1551/0x1ff0 net/9p/client.c:1031 v9fs_session_init+0x1b9/0x28e0 fs/9p/v9fs.c:410 v9fs_mount+0xe2/0x12b0 fs/9p/vfs_super.c:122 legacy_get_tree+0x114/0x290 fs/fs_context.c:662 vfs_get_tree+0xa7/0x570 fs/super.c:1797 do_new_mount+0x71f/0x15e0 fs/namespace.c:3352 path_mount+0x742/0x1f20 fs/namespace.c:3679 do_mount fs/namespace.c:3692 [inline] __do_sys_mount fs/namespace.c:3898 [inline] __se_sys_mount+0x725/0x810 fs/namespace.c:3875 __x64_sys_mount+0xe4/0x150 fs/namespace.c:3875 do_syscall_64+0xd5/0x1f0 entry_SYSCALL_64_after_hwframe+0x6d/0x75 If p9_check_errors() fails early in p9_client_rpc(), req->rc.tag will not be properly initialized. However, trace_9p_client_res() ends up trying to print it out anyway before p9_client_rpc() finishes. Fix this issue by assigning default values to p9_fcall fields such as 'tag' and (just in case KMSAN unearths something new) 'id' during the tag allocation stage. Reported-and-tested-by: syzbot+ff14db38f56329ef68df@syzkaller.appspotmail.com Fixes: 348b59012e5c ("net/9p: Convert net/9p protocol dumps to tracepoints") Signed-off-by: Nikita Zhandarovich Reviewed-by: Christian Schoenebeck Cc: stable@vger.kernel.org Message-ID: <20240408141039.30428-1-n.zhandarovich@fintech.ru> Signed-off-by: Dominique Martinet

netfs, 9p: Implement helpers for new write code

2024-05-01T17:07:37+00:00

Implement the helpers for the new write code in 9p. There's now an optional ->prepare_write() that allows the filesystem to set the parameters for the next write, such as maximum size and maximum segment count, and an ->issue_write() that is called to initiate an (asynchronous) write operation. Signed-off-by: David Howells Reviewed-by: Jeff Layton cc: Eric Van Hensbergen cc: Latchesar Ionkov cc: Dominique Martinet cc: Christian Schoenebeck cc: v9fs@lists.linux.dev cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org

9p: Fix read/write debug statements to report server reply

2024-02-12T12:18:54+00:00

Previous conversion to iov missed these debug statements which would now always print the requested size instead of the actual server reply. Write also added a loop in a much older commit but we didn't report these, while reads do report each iteration -- it's more coherent to keep reporting all requests to server so move that at the same time. Fixes: 7f02464739da ("9p: convert to advancing variant of iov_iter_get_pages_alloc()") Signed-off-by: Dominique Martinet Message-ID: <20240109-9p-rw-trace-v1-1-327178114257@codewreck.org>

9p/net: fix possible memory leak in p9_check_errors()

2023-10-27T03:44:13+00:00

When p9pdu_readf() is called with "s?d" attribute, it allocates a pointer that will store a string. But when p9pdu_readf() fails while handling "d" then this pointer will not be freed in p9_check_errors(). Fixes: 51a87c552dfd ("9p: rework client code to use new protocol support functions") Reviewed-by: Christian Schoenebeck Signed-off-by: Hangyu Hua Message-ID: <20231027030302.11927-1-hbh25y@gmail.com> Signed-off-by: Dominique Martinet

9p: v9fs_listxattr: fix %s null argument warning

2023-10-25T22:05:52+00:00

W=1 warns about null argument to kprintf: In file included from fs/9p/xattr.c:12: In function ‘v9fs_xattr_get’, inlined from ‘v9fs_listxattr’ at fs/9p/xattr.c:142:9: include/net/9p/9p.h:55:2: error: ‘%s’ directive argument is null [-Werror=format-overflow=] 55 | _p9_debug(level, __func__, fmt, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Use an empty string instead of : - this is ok 9p-wise because p9pdu_vwritef serializes a null string and an empty string the same way (one '0' word for length) - since this degrades the print statements, add new single quotes for xattr's name delimter (Old: "file = (null)", new: "file = ''") Link: https://lore.kernel.org/r/20231008060138.517057-1-suhui@nfschina.com Suggested-by: Su Hui Signed-off-by: Dominique Martinet Acked-by: Christian Schoenebeck Message-ID: <20231025103445.1248103-2-asmadeus@codewreck.org>

9p: remove dead stores (variable set again without being read)

2023-07-20T19:14:50+00:00

The 9p code for some reason used to initialize variables outside of the declaration, e.g. instead of just initializing the variable like this: int retval = 0 We would be doing this: int retval; retval = 0; This is perfectly fine and the compiler will just optimize dead stores anyway, but scan-build seems to think this is a problem and there are many of these warnings making the output of scan-build full of such warnings: fs/9p/vfs_inode.c:916:2: warning: Value stored to 'retval' is never read [deadcode.DeadStores] retval = 0; ^ ~ I have no strong opinion here, but if we want to regularly run scan-build we should fix these just to silence the messages. I've confirmed these all are indeed ok to remove. Reviewed-by: Simon Horman Signed-off-by: Dominique Martinet Signed-off-by: Eric Van Hensbergen