From: Chuck Lever chuck.lever@oracle.com
Testing shows that the EBUSY error return from mtree_alloc_cyclic() leaks into user space. The ERRORS section of "man creat(2)" says:
EBUSY O_EXCL was specified in flags and pathname refers to a block device that is in use by the system (e.g., it is mounted).
ENOSPC is closer to what applications expect in this situation.
Note that the normal range of simple directory offset values is 2..2^63, so hitting this error is going to be rare to impossible.
Fixes: 6faddda69f62 ("libfs: Add directory operations for stable offsets") Cc: stable@vger.kernel.org # v6.9+ Reviewed-by: Jeff Layton jlayton@kernel.org Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Chuck Lever chuck.lever@oracle.com --- fs/libfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/libfs.c b/fs/libfs.c index 748ac5923154..f6d04c69f195 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -292,7 +292,9 @@ int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry)
ret = mtree_alloc_cyclic(&octx->mt, &offset, dentry, DIR_OFFSET_MIN, LONG_MAX, &octx->next_offset, GFP_KERNEL); - if (ret < 0) + if (unlikely(ret == -EBUSY)) + return -ENOSPC; + if (unlikely(ret < 0)) return ret;
offset_set(dentry, offset);
From: cel@kernel.org
Sent: 15 December 2024 18:58
From: Chuck Lever chuck.lever@oracle.com
Testing shows that the EBUSY error return from mtree_alloc_cyclic() leaks into user space. The ERRORS section of "man creat(2)" says:
EBUSY O_EXCL was specified in flags and pathname refers to a block device that is in use by the system (e.g., it is mounted).
ENOSPC is closer to what applications expect in this situation.
Note that the normal range of simple directory offset values is 2..2^63, so hitting this error is going to be rare to impossible.
Fixes: 6faddda69f62 ("libfs: Add directory operations for stable offsets") Cc: stable@vger.kernel.org # v6.9+ Reviewed-by: Jeff Layton jlayton@kernel.org Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Chuck Lever chuck.lever@oracle.com
fs/libfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/libfs.c b/fs/libfs.c index 748ac5923154..f6d04c69f195 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -292,7 +292,9 @@ int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry)
ret = mtree_alloc_cyclic(&octx->mt, &offset, dentry, DIR_OFFSET_MIN, LONG_MAX, &octx->next_offset, GFP_KERNEL);
- if (ret < 0)
- if (unlikely(ret == -EBUSY))
return -ENOSPC;- if (unlikely(ret < 0)) return ret;
You've just added an extra comparison to a hot path. Doing: if (ret < 0) return ret == -EBUSY ? -ENOSPC : ret; would be better.
David
offset_set(dentry, offset);
2.47.0
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Sun, Dec 15 2024, David Laight wrote:
From: cel@kernel.org
Sent: 15 December 2024 18:58
From: Chuck Lever chuck.lever@oracle.com
Testing shows that the EBUSY error return from mtree_alloc_cyclic() leaks into user space. The ERRORS section of "man creat(2)" says:
EBUSY O_EXCL was specified in flags and pathname refers to a block device that is in use by the system (e.g., it is mounted).
ENOSPC is closer to what applications expect in this situation.
Note that the normal range of simple directory offset values is 2..2^63, so hitting this error is going to be rare to impossible.
Fixes: 6faddda69f62 ("libfs: Add directory operations for stable offsets") Cc: stable@vger.kernel.org # v6.9+ Reviewed-by: Jeff Layton jlayton@kernel.org Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Chuck Lever chuck.lever@oracle.com
fs/libfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/libfs.c b/fs/libfs.c index 748ac5923154..f6d04c69f195 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -292,7 +292,9 @@ int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry)
ret = mtree_alloc_cyclic(&octx->mt, &offset, dentry, DIR_OFFSET_MIN, LONG_MAX, &octx->next_offset, GFP_KERNEL);
- if (ret < 0)
- if (unlikely(ret == -EBUSY))
return -ENOSPC;- if (unlikely(ret < 0)) return ret;
You've just added an extra comparison to a hot path. Doing: if (ret < 0) return ret == -EBUSY ? -ENOSPC : ret; would be better.
This also has two comparisons: one for ret < 0 and another for ret == -EBUSY. So I don't see a difference. I was curious to see if compilers can somehow optimize one or the other, so I ran the two on godbolt and I see no real difference between the two: https://godbolt.org/z/9Gav6b6Mf
On 12/16/24 8:39 AM, Pratyush Yadav wrote:
On Sun, Dec 15 2024, David Laight wrote:
From: cel@kernel.org
Sent: 15 December 2024 18:58
From: Chuck Lever chuck.lever@oracle.com
Testing shows that the EBUSY error return from mtree_alloc_cyclic() leaks into user space. The ERRORS section of "man creat(2)" says:
EBUSY O_EXCL was specified in flags and pathname refers to a block device that is in use by the system (e.g., it is mounted).
ENOSPC is closer to what applications expect in this situation.
Note that the normal range of simple directory offset values is 2..2^63, so hitting this error is going to be rare to impossible.
Fixes: 6faddda69f62 ("libfs: Add directory operations for stable offsets") Cc: stable@vger.kernel.org # v6.9+ Reviewed-by: Jeff Layton jlayton@kernel.org Reviewed-by: Yang Erkun yangerkun@huawei.com Signed-off-by: Chuck Lever chuck.lever@oracle.com
fs/libfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/libfs.c b/fs/libfs.c index 748ac5923154..f6d04c69f195 100644 --- a/fs/libfs.c +++ b/fs/libfs.c @@ -292,7 +292,9 @@ int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry)
ret = mtree_alloc_cyclic(&octx->mt, &offset, dentry, DIR_OFFSET_MIN, LONG_MAX, &octx->next_offset, GFP_KERNEL);
- if (ret < 0)
- if (unlikely(ret == -EBUSY))
return -ENOSPC;- if (unlikely(ret < 0)) return ret;
You've just added an extra comparison to a hot path. Doing: if (ret < 0) return ret == -EBUSY ? -ENOSPC : ret; would be better.
This also has two comparisons: one for ret < 0 and another for ret == -EBUSY. So I don't see a difference. I was curious to see if compilers can somehow optimize one or the other, so I ran the two on godbolt and I see no real difference between the two: https://godbolt.org/z/9Gav6b6Mf
In my version, both comparisons are done every time through this flow. David's version changes it so that only one comparison is done unless @ret is less than zero (which is rare).
I've updated simple_offset_add() in my tree to use David's version.
linux-stable-mirror@lists.linaro.org
-
cel＠kernel.org -
Chuck Lever -
David Laight -
Pratyush Yadav