casaroli commented on code in PR #20593:
URL: https://github.com/apache/nuttx/pull/20593#discussion_r4248100579
##########
fs/v9fs/v9fs.c:
##########
@@ -150,7 +157,16 @@ const struct mountpt_operations g_v9fs_operations =
v9fs_vfs_rmdir, /* rmdir */
v9fs_vfs_rename, /* rename */
v9fs_vfs_stat, /* stat */
- v9fs_vfs_chstat /* chstat */
+ v9fs_vfs_chstat, /* chstat */
+ NULL, /* syncfs */
+ NULL, /* ioctldir */
+ NULL, /* permission */
+#ifdef CONFIG_FS_LINKS
+ NULL, /* link */
Review Comment:
Done: I cherry-picked 5eec81b as the first commit and dropped my walk and
fid commits, which it covers.
On master it needed two changes: the operations table needs the ioctldir and
permission slots (the four link operations went into the wrong slots), and the
buffers use lib_get_pathbuffer().
I added three small fixes on top. open() keeps the walk error (ELOOP was
ENOENT) and creates only on ENOENT. readlink() truncates instead of returning
ENAMETOOLONG and keeps its reply off the stack. symlink() and link() check the
name lengths.
The last one is a real crash: the Tsymlink request has room for only 2 *
NAME_MAX bytes, and `ln -s` with a 68-byte target panics the kernel (the return
address is bytes of the target).
Tested on qemu-armv8a:netnsh with a 9P share; the results are in the PR
description.
##########
fs/v9fs/client.c:
##########
@@ -2381,38 +2374,56 @@ ssize_t v9fs_client_readlink(FAR struct v9fs_client_s
*client, uint32_t fid,
FAR char *buffer, size_t buflen)
{
struct v9fs_readlink_s request;
- struct v9fs_rreadlink_s response;
+ FAR char *response;
struct iovec wiov[1];
struct iovec riov[1];
+ uint16_t len;
int ret;
/* size[4] Treadlink tag[2] fid[4]
* size[4] Rreadlink tag[2] target[s]
+ *
+ * The response is too big for the stack: use a path buffer.
*/
+ response = lib_get_pathbuffer();
+ if (response == NULL)
+ {
+ return -ENOMEM;
+ }
+
request.fid = fid;
request.header.size = V9FS_HDRSZ + V9FS_BIT32SZ;
request.header.type = V9FS_TREADLINK;
request.header.tag = v9fs_get_tagid(client);
wiov[0].iov_base = &request;
wiov[0].iov_len = V9FS_HDRSZ + V9FS_BIT32SZ;
- riov[0].iov_base = &response;
- riov[0].iov_len = sizeof(response);
+ riov[0].iov_base = response;
+ riov[0].iov_len = PATH_MAX;
ret = v9fs_client_rpc(client->transport, wiov, 1, riov, 1,
request.header.tag);
if (ret < 0)
{
- return ret;
+ goto out;
}
- if (response.target_len > buflen)
+ memcpy(&len, &response[V9FS_HDRSZ], V9FS_BIT16SZ);
Review Comment:
Done: v9fs_rreadlink_s is back, with target[] as a flexible array.
##########
fs/v9fs/client.c:
##########
@@ -2293,42 +2293,61 @@ int v9fs_client_symlink(FAR struct v9fs_client_s
*client, uint32_t fid,
FAR const char *name, FAR const char *symtgt,
int mode)
{
- struct v9fs_symlink_s request;
+ FAR struct v9fs_symlink_s *request;
struct v9fs_rsymlink_s response;
struct iovec wiov[1];
struct iovec riov[1];
uint32_t gid = getgid();
off_t offset = 0;
uint16_t symtgt_len;
+ size_t name_len;
+ uint16_t tag;
+ int ret;
/* size[4] Tsymlink tag[2] fid[4] name[s] symtgt[s] gid[4]
* size[4] Rsymlink tag[2] qid[13]
*/
- request.fid = fid;
- request.name_len = strlen(name);
- symtgt_len = strlen(symtgt);
- request.header.size = V9FS_HDRSZ + V9FS_BIT32SZ * 2 + V9FS_BIT16SZ * 2 +
- request.name_len + symtgt_len;
- request.header.type = V9FS_TSYMLINK;
- request.header.tag = v9fs_get_tagid(client);
+ name_len = strlen(name);
+ symtgt_len = strnlen(symtgt, PATH_MAX);
+ if (name_len > NAME_MAX || symtgt_len >= PATH_MAX)
+ {
+ return -ENAMETOOLONG;
+ }
- memcpy(&request.buffer[offset], name, request.name_len);
- offset += request.name_len;
- memcpy(&request.buffer[offset], &symtgt_len, V9FS_BIT16SZ);
+ /* The request is too big for the stack */
+
+ request = (FAR struct v9fs_symlink_s *)
+ lib_get_tempbuffer(sizeof(struct v9fs_symlink_s));
+ if (request == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ tag = v9fs_get_tagid(client);
+ request->fid = fid;
+ request->name_len = name_len;
+ request->header.size = V9FS_HDRSZ + V9FS_BIT32SZ * 2 +
+ V9FS_BIT16SZ * 2 + name_len + symtgt_len;
+ request->header.type = V9FS_TSYMLINK;
+ request->header.tag = tag;
+
+ memcpy(&request->buffer[offset], name, name_len);
+ offset += name_len;
+ memcpy(&request->buffer[offset], &symtgt_len, V9FS_BIT16SZ);
offset += V9FS_BIT16SZ;
- memcpy(&request.buffer[offset], symtgt, symtgt_len);
+ memcpy(&request->buffer[offset], symtgt, symtgt_len);
offset += symtgt_len;
- memcpy(&request.buffer[offset], &gid, V9FS_BIT32SZ);
+ memcpy(&request->buffer[offset], &gid, V9FS_BIT32SZ);
- wiov[0].iov_base = &request;
- wiov[0].iov_len = V9FS_HDRSZ + V9FS_BIT32SZ * 2 + V9FS_BIT16SZ * 2 +
- request.name_len + symtgt_len;
+ wiov[0].iov_base = request;
+ wiov[0].iov_len = request->header.size;
riov[0].iov_base = &response;
riov[0].iov_len = V9FS_HDRSZ + V9FS_QIDSZ;
- return v9fs_client_rpc(client->transport, wiov, 1, riov, 1,
- request.header.tag);
+ ret = v9fs_client_rpc(client->transport, wiov, 1, riov, 1, tag);
+ lib_put_tempbuffer((FAR char *)request);
Review Comment:
Done: lib_put_tempbuffer() takes char *, so the function keeps the char * it
got from lib_get_tempbuffer() and gives that back. The struct pointer is cast
once, where the buffer is taken.
##########
fs/v9fs/client.c:
##########
@@ -2381,38 +2374,56 @@ ssize_t v9fs_client_readlink(FAR struct v9fs_client_s
*client, uint32_t fid,
FAR char *buffer, size_t buflen)
{
struct v9fs_readlink_s request;
- struct v9fs_rreadlink_s response;
+ FAR char *response;
struct iovec wiov[1];
struct iovec riov[1];
+ uint16_t len;
int ret;
/* size[4] Treadlink tag[2] fid[4]
* size[4] Rreadlink tag[2] target[s]
+ *
+ * The response is too big for the stack: use a path buffer.
*/
+ response = lib_get_pathbuffer();
Review Comment:
Done: it uses lib_get_tempbuffer() now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
