You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "casaroli (via GitHub)" <gi...@apache.org> on 2026/10/11 18:57:03 UTC
[PR] fs/v9fs: Follow symbolic links, and add readlink() and lstat(). [nuttx]
casaroli opened a new pull request, #20593:
URL: https://github.com/apache/nuttx/pull/20593
## Summary
The 9P server does not walk through a symbolic link and does not open one. On a v9fs mount, a path with a link in it was not found, a link to a file could not be opened, and `readlink()` and `lstat()` were not supported. Also, every failed lookup left a fid in the client's fid table.
This PR has three commits:
- Free the fid of a walk that does not reach the last name. The server creates the new fid only when it walks all names.
- Follow symbolic links. When a walk ends at a link, the client reads it with `Treadlink`, puts the target in place of the link and walks again, up to `SYMLOOP_MAX` links (then `ELOOP`). The server walks the `.` and `..` names of the target inside the exported tree. Links on the way are always followed; the last name is followed by `open()` (unless `O_NOFOLLOW`), `opendir()`, `stat()` and `chstat()`, and not by `unlink()`, `rmdir()` and `rename()`, which act on the link. An absolute target names a file outside of the file system and is not found. `open()` now returns the error of the walk, such as `ELOOP`, instead of `ENOENT`, and creates a file only when the walk did not find it.
- With `CONFIG_FS_LINKS`, add `readlink()` and `lstat()`.
## Impact
v9fs mounts with symbolic links work. `rm` of a link removes the link, not its target. No change for shares without links.
## Testing
The run-time tests ran on a local branch that has this change and other pending changes on top of master; the builds are of this branch alone.
qemu-armv8a, kernel build, QEMU 11 with HVF on Apple M4, a 9P share (`security_model=none`) with links made on the host:
```
cat flink (flink -> d/sub/f.txt) hello
cat dlink/sub/f.txt (dlink -> d) hello
cat chain (chain -> flink) hello
cat uplink (uplink -> ../lt/d/sub/f.txt) hello
cat dlink/sublink/f.txt (d/sublink -> sub) hello
cat loop1 (loop1 -> loop2 -> loop1) Too many symbolic links encountered
cat abslink (abslink -> /etc/hosts) No such file or directory
rm rmlink; cat target.txt (rmlink -> target.txt) keep (the link is gone, the target stays)
wc -l < tools/syscall_64.tbl (link to ../../../scripts/syscall.tbl) 414
readlink flink dlink uplink d/sub/f.txt d ../lt/d/sub/f.txt
test -L flink, test -L target.txt link, not a link
ls -l flink; stat -c %F flink; stat -L -c %F flink lrwxr-xr-x, symbolic link, regular file
```
Builds with `-Wno-cpp -Werror`: `qemu-armv8a:netnsh`, `qemu-armv8a:netnsh` with `CONFIG_FS_LINKS=y`, `qemu-armv8a:fastboot`. `tools/checkpatch.sh -c -u -m -g` passes.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fs/v9fs: Follow symbolic links, and add readlink() and lstat(). [nuttx]
Posted by "casaroli (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fs/v9fs: Follow symbolic links, and add readlink() and lstat(). [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20593:
URL: https://github.com/apache/nuttx/pull/20593#discussion_r4244153820
##########
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:
could you cherry-pick https://github.com/xiaoxiang781216/nuttx/commit/5eec81b720d23c713c761ddff055c59887ff97d1
which contain more implementation (e.g. link/symlink)?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fs/v9fs: Follow symbolic links, and add readlink() and lstat(). [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #20593:
URL: https://github.com/apache/nuttx/pull/20593
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fs/v9fs: Follow symbolic links, and add readlink() and lstat(). [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #20593:
URL: https://github.com/apache/nuttx/pull/20593#discussion_r4244492904
##########
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:
call lib_get_tempbuffer instead
##########
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:
should we keep v9fs_rreadlink_s, but change it to:
```
begin_packed_struct struct v9fs_rreadlink_s
{
struct v9fs_header_s header;
uint16_t target_len;
uint8_t target[];
} end_packed_struct;
```
to simplify the following code.
##########
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:
remove the cast
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fs/v9fs: Follow symbolic links, and add readlink() and lstat(). [nuttx]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #20593:
URL: https://github.com/apache/nuttx/pull/20593#issuecomment-6112589432
<!-- membrowse-pr-comment -->
## <img src="https://membrowse.com/membrowse-logo.svg" height="24" align="top"> MemBrowse Memory Report
*No memory changes detected for:*
- [arduino-mega2560](https://membrowse.com/public/apache/nuttx/target/arduino-mega2560/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [esp32-devkitc](https://membrowse.com/public/apache/nuttx/target/esp32-devkitc/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [hifive1-revb](https://membrowse.com/public/apache/nuttx/target/hifive1-revb/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [mirtoo](https://membrowse.com/public/apache/nuttx/target/mirtoo/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [qemu-armv8a](https://membrowse.com/public/apache/nuttx/target/qemu-armv8a/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [qemu-intel64](https://membrowse.com/public/apache/nuttx/target/qemu-intel64/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [rx65n-rsk2mb](https://membrowse.com/public/apache/nuttx/target/rx65n-rsk2mb/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [s698pm-dkit](https://membrowse.com/public/apache/nuttx/target/s698pm-dkit/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
- [stm32-nucleo-f103rb](https://membrowse.com/public/apache/nuttx/target/stm32-nucleo-f103rb/compare?head=a574c4b66b1468c301032abf60fe12380c0cc6b7&base=4bc1ab2fd7bede547e91c8e3a7dac3bd6eb941f7)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org