You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/25 16:48:04 UTC
[GitHub] [incubator-nuttx] GUIDINGLI opened a new pull request #5844: rpmsgfs: do NOT access the pointer when do remote ioctl
GUIDINGLI opened a new pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844
## Summary
rpmsgfs: do NOT access the pointer when do remote ioctl
rpmsgfs/rename: fix bug about pathname align with 8bytes
## Impact
rpmsgfs
## Testing
VELA
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5844: rpmsgfs: do NOT access the pointer when do remote ioctl
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5844: rpmsgfs: do NOT access the pointer when do remote ioctl
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844#discussion_r836513791
##########
File path: fs/rpmsgfs/rpmsgfs.h
##########
@@ -110,7 +110,9 @@ begin_packed_struct struct rpmsgfs_ioctl_s
struct rpmsgfs_header_s header;
int32_t fd;
int32_t request;
- int32_t arg;
+ uint64_t arg;
+ uint32_t arglen;
+ char buf[0];
Review comment:
Sure. @liguiding please provide a new PR change all buf[0] to buf[]
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5844: rpmsgfs: do NOT access the pointer when do remote ioctl
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844#issuecomment-1083524657
@GUIDINGLI please rebase your patch to fix the ci error
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on pull request #5844: rpmsgfs: do NOT access the pointer when do remote ioctl
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844#issuecomment-1081860088
```
rpmsgfs/rpmsgfs_client.c: In function 'rpmsgfs_ioctl_handler':
Error: rpmsgfs/rpmsgfs_client.c:176:3: error: implicit declaration of function 'rpmsg_post' [-Werror=implicit-function-declaration]
176 | rpmsg_post(ept, &cookie->sem);
| ^~~~~~~~~~
rpmsgfs/rpmsgfs_client.c: In function 'rpmsgfs_client_ioctl':
Error: rpmsgfs/rpmsgfs_client.c:506:9: error: implicit declaration of function 'rpmsgfs_get_tx_payload_buffer'; did you mean 'rpmsg_get_tx_payload_buffer'? [-Werror=implicit-function-declaration]
506 | msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| rpmsg_get_tx_payload_buffer
Error: rpmsgfs/rpmsgfs_client.c:506:7: error: assignment to 'struct rpmsgfs_ioctl_s *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
506 | msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
| ^
```
Please fix the build
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5844: rpmsgfs: do NOT access the pointer when do remote ioctl
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844#discussion_r836513791
##########
File path: fs/rpmsgfs/rpmsgfs.h
##########
@@ -110,7 +110,9 @@ begin_packed_struct struct rpmsgfs_ioctl_s
struct rpmsgfs_header_s header;
int32_t fd;
int32_t request;
- int32_t arg;
+ uint64_t arg;
+ uint32_t arglen;
+ char buf[0];
Review comment:
Sure.
--
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
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5844: rpmsgfs: do NOT access the pointer when do remote ioctl
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5844:
URL: https://github.com/apache/incubator-nuttx/pull/5844#discussion_r835784550
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -460,15 +496,34 @@ off_t rpmsgfs_client_lseek(FAR void *handle, int fd,
int rpmsgfs_client_ioctl(FAR void *handle, int fd,
int request, unsigned long arg)
{
- struct rpmsgfs_ioctl_s msg =
- {
- .fd = fd,
- .request = request,
- .arg = arg,
- };
+ size_t arglen = rpmsgfs_ioctl_arglen(request);
+ FAR struct rpmsgfs_s *priv = handle;
+ FAR struct rpmsgfs_ioctl_s *msg;
+ uint32_t space;
+ size_t len;
- return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, true,
- (struct rpmsgfs_header_s *)&msg, sizeof(msg), NULL);
+ len = sizeof(*msg) + arglen;
+ msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
+ if (!msg)
Review comment:
```suggestion
if (msg == NULL)
```
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -460,15 +496,34 @@ off_t rpmsgfs_client_lseek(FAR void *handle, int fd,
int rpmsgfs_client_ioctl(FAR void *handle, int fd,
int request, unsigned long arg)
{
- struct rpmsgfs_ioctl_s msg =
- {
- .fd = fd,
- .request = request,
- .arg = arg,
- };
+ size_t arglen = rpmsgfs_ioctl_arglen(request);
+ FAR struct rpmsgfs_s *priv = handle;
+ FAR struct rpmsgfs_ioctl_s *msg;
+ uint32_t space;
+ size_t len;
- return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, true,
- (struct rpmsgfs_header_s *)&msg, sizeof(msg), NULL);
+ len = sizeof(*msg) + arglen;
+ msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
+ if (!msg)
+ {
+ return -ENOMEM;
+ }
+
+ DEBUGASSERT(len <= space);
+
+ msg->fd = fd;
+ msg->request = request;
+ msg->arg = arg;
+ msg->arglen = arglen;
+
+ if (arglen > 0)
+ {
+ memcpy(msg->buf, (void *)(uintptr_t)arg, arglen);
+ }
+
+ return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, false,
+ (struct rpmsgfs_header_s *)msg, len,
+ arglen > 0 ? (void *)arg : NULL);
Review comment:
please add `FAR` to pointer casts
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -155,6 +159,25 @@ static int rpmsgfs_read_handler(FAR struct rpmsg_endpoint *ept,
return 0;
}
+static int rpmsgfs_ioctl_handler(FAR struct rpmsg_endpoint *ept,
+ FAR void *data, size_t len,
+ uint32_t src, FAR void *priv)
+{
+ FAR struct rpmsgfs_header_s *header = data;
+ FAR struct rpmsgfs_cookie_s *cookie =
+ (struct rpmsgfs_cookie_s *)(uintptr_t)header->cookie;
Review comment:
```suggestion
(FAR struct rpmsgfs_cookie_s *)(uintptr_t)header->cookie;
```
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -155,6 +159,25 @@ static int rpmsgfs_read_handler(FAR struct rpmsg_endpoint *ept,
return 0;
}
+static int rpmsgfs_ioctl_handler(FAR struct rpmsg_endpoint *ept,
+ FAR void *data, size_t len,
+ uint32_t src, FAR void *priv)
+{
+ FAR struct rpmsgfs_header_s *header = data;
+ FAR struct rpmsgfs_cookie_s *cookie =
+ (struct rpmsgfs_cookie_s *)(uintptr_t)header->cookie;
+ FAR struct rpmsgfs_ioctl_s *rsp = data;
+
+ if (cookie->result >= 0 && rsp->arglen > 0)
+ {
+ memcpy(cookie->data, (void *)(uintptr_t)rsp->arg, rsp->arglen);
Review comment:
```suggestion
memcpy(cookie->data, (FAR void *)(uintptr_t)rsp->arg, rsp->arglen);
```
##########
File path: fs/rpmsgfs/rpmsgfs_client.c
##########
@@ -460,15 +496,34 @@ off_t rpmsgfs_client_lseek(FAR void *handle, int fd,
int rpmsgfs_client_ioctl(FAR void *handle, int fd,
int request, unsigned long arg)
{
- struct rpmsgfs_ioctl_s msg =
- {
- .fd = fd,
- .request = request,
- .arg = arg,
- };
+ size_t arglen = rpmsgfs_ioctl_arglen(request);
+ FAR struct rpmsgfs_s *priv = handle;
+ FAR struct rpmsgfs_ioctl_s *msg;
+ uint32_t space;
+ size_t len;
- return rpmsgfs_send_recv(handle, RPMSGFS_IOCTL, true,
- (struct rpmsgfs_header_s *)&msg, sizeof(msg), NULL);
+ len = sizeof(*msg) + arglen;
+ msg = rpmsgfs_get_tx_payload_buffer(priv, &space);
+ if (!msg)
+ {
+ return -ENOMEM;
+ }
+
+ DEBUGASSERT(len <= space);
+
+ msg->fd = fd;
+ msg->request = request;
+ msg->arg = arg;
+ msg->arglen = arglen;
+
+ if (arglen > 0)
+ {
+ memcpy(msg->buf, (void *)(uintptr_t)arg, arglen);
Review comment:
```suggestion
memcpy(msg->buf, (FAR void *)(uintptr_t)arg, arglen);
```
##########
File path: fs/rpmsgfs/rpmsgfs.h
##########
@@ -110,7 +110,9 @@ begin_packed_struct struct rpmsgfs_ioctl_s
struct rpmsgfs_header_s header;
int32_t fd;
int32_t request;
- int32_t arg;
+ uint64_t arg;
+ uint32_t arglen;
+ char buf[0];
Review comment:
@xiaoxiang781216 can we switch to true C99 `buf[];` instead of GCC extension `buf[0];` if we really would like to use flexible array members in the code?
--
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