catalinv-ncc commented on PR #19476:
URL: https://github.com/apache/nuttx/pull/19476#issuecomment-5016476716
> Are we sure the upper-half fs driver doesn't perform a null check? That
would be a better place for this.
The answer is no, there is no check (at least in some cases), but you are
raising a very good point.
```c
ssize_t file_readv(FAR struct file *filep,
FAR const struct iovec *iov, int iovcnt)
{
...
#if !defined(CONFIG_BUILD_KERNEL) || CONFIG_ARCH_TEXT_VBASE != 0
for (ret = 0; ret < iovcnt; ret++)
{
if (iov[ret].iov_base == NULL && iov[ret].iov_len != 0)
{
return -EFAULT;
}
}
#endif
```
Fundamentally, the above get to execute when calling `rea()` from user
space. However, see the compilation flags. In some cases it is checked, in some
it is not. See boards/arm64/a527/avaota-a1/configs/nsh/defconfig.
POSIX spec has no specification wrt read() and providing a NULL pointer, so
undefined behavior. There is no reason not to restrict passing of a NULL
pointer to read at any time, so I suggest following your recommendation
On second thought, as a better solution, I would remove my initial
conditions, and I would change this:
```c
#if !defined(CONFIG_BUILD_KERNEL) || CONFIG_ARCH_TEXT_VBASE != 0
for (ret = 0; ret < iovcnt; ret++)
{
if (iov[ret].iov_base == NULL && iov[ret].iov_len != 0)
{
return -EFAULT;
}
}
#endif
```
to this:
```c
for (ret = 0; ret < iovcnt; ret++)
{
if (iov[ret].iov_base == NULL)
{
return -EFAULT;
}
}
```
Basically, not allow NULL buffers under any circumstances. Any thougths
@linguini1 ?
--
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]
