On Wed, Aug 06, 2025 at 03:31:10PM +1000, Aleksa Sarai wrote:
> Userspace generally expects APIs that return EMSGSIZE to allow for them
> to adjust their buffer size and retry the operation. However, the
> fscontext log would previously clear the message even in the EMSGSIZE
> case.
>
> Given that it is very cheap for us to check whether the buffer is too
> small before we remove the message from the ring buffer, let's just do
> that instead.
>
> Fixes: 007ec26cdc9f ("vfs: Implement logging through fs_context")
> Cc: David Howells <[email protected]>
> Cc: <[email protected]> # v5.2+
> Signed-off-by: Aleksa Sarai <[email protected]>
> ---
> fs/fsopen.c | 22 ++++++++++++++--------
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fsopen.c b/fs/fsopen.c
> index 1aaf4cb2afb2..f5fdaa97965b 100644
> --- a/fs/fsopen.c
> +++ b/fs/fsopen.c
> @@ -36,23 +36,25 @@ static ssize_t fscontext_read(struct file *file,
> if (ret < 0)
> return ret;
>
> - if (log->head == log->tail) {
> - mutex_unlock(&fc->uapi_mutex);
> - return -ENODATA;
> - }
> + ret = -ENODATA;
> + if (log->head == log->tail)
> + goto err_unlock_nomsg;
>
> index = log->tail & (logsize - 1);
> p = log->buffer[index];
> + n = strlen(p);
> +
> + ret = -EMSGSIZE;
> + if (n > len)
> + goto err_unlock_nomsg;
> +
FWIW, I would rather turn that into a helper taking log and len and
returning p or ERR_PTR(...); something like
static inline const char *fetch_message(struct fc_log *log, size_t len,
bool *need_free)
{
int index = log->tail & (ARRAY_SIZE(log->buffer) - 1);
const char *p = log->buffer[index];
if (unlikely(log->head == log->tail))
return ERR_PTR(-ENODATA);
n = strlen(p);
if (unlikely(n > len))
return ERR_PTR(-EMSGSIZE);
log->buffer[index] = NULL;
*need_free = log->need_free & (1 << index);
log->need_free &= ~(1 << index);
log->tail++;
return p;
}
with caller being
ret = mutex_lock_interruptible(&fc->uapi_mutex);
if (ret < 0)
return ret;
p = fetch_message(log, len, &need_free);
mutex_unlock(&fc->uapi_mutex);
if (IS_ERR(p))
return PTR_ERR(p);
n = strlen(p);
if (copy_to_user(_buf, p, n))
n = -EFAULT;
if (need_free)
kfree(p);
return n;
and that's it.
