I'm new here, first time reporting a regression, apologies in advance if I'm doing something wrong of if this was already reported (I found some CIFS issues but not exactly this one).
I'm using x86-64 Arch Linux and LTS kernel (6.1.71 as I write this) and I noticed a regression that I could reproduce in other boxes with other architectures as well (aarch64 with 6.1.70).
# mount.cifs //server/share /mnt # mount //server/share on /mnt type cifs (rw,relatime,vers=3.1.1...) # cd /mnt # df . df: .: Resource temporarily unavailable # ls -al ls: .: Resource temporarily unavailable ls: file1: Resource temporarily unavailable ls: file2: Resource temporarily unavailable [...then ls shows the listing...]
If I use strace with df, the problem is: statfs(".", 0x.....) = -1 EAGAIN (Resource temporarily unavailable)
And with ls: listxattr(".", 0x..., 152): -1 EAGAIN (Resource temporarily unavailable) listxattr("file1", ..., 152): -1 EAGAIN (same as above) ...
Initially I thought the problem was with the Samba server and/or the client mount flags, but I've spent a day trying a *lot* of different combinations and nothing worked. This happens with any share that I try, and I've tried mounting shares from multiple Linux boxes running different Samba and kernel versions.
Then I tried changing kernel versions at my client box. I booted latest 6.6.9 and the problem simply disappeared. My Debian server with 6.5.11 also doesn't have it. I then started a VM and tried a "bisection" of 6.1.x versions, leading to kernel 6.1.70 when this started to happen. 6.1.69 and older look fine.
I hope that this is enough information to reproduce this issue. I will be glad to provide more info if necessary.
// Leonardo.
Hi,
I confirm Leonardo's findings about 6.1.70 introducing this regression, this issue manifested in Home Assistant OS [1] which was recently bumped to that version. I bisected the issue between 6.1.69 and 6.1.70 which pointed me to this bad commit:
---- commit bef4315f19ba6f434054f58b958c0cf058c7a43f (refs/bisect/bad) Author: Paulo Alcantara pc@manguebit.com Date: Wed Dec 13 12:25:57 2023 -0300
smb: client: fix OOB in SMB2_query_info_init()
commit 33eae65c6f49770fec7a662935d4eb4a6406d24b upstream.
A small CIFS buffer (448 bytes) isn't big enough to hold SMB2_QUERY_INFO request along with user's input data from CIFS_QUERY_INFO ioctl. That is, if the user passed an input buffer > 344 bytes, the client will memcpy() off the end of @req->Buffer in SMB2_query_info_init() thus causing the following KASAN splat:
(snip...) ----
Reverting this change on 6.1.y makes the error go away.
Adding linux-cifs and Paolo to CC.
Cheers, Jan
[1] https://github.com/home-assistant/operating-system/issues/3041
On 08. 01. 24 11:44, Leonardo Brondani Schenkel wrote:
I'm new here, first time reporting a regression, apologies in advance if I'm doing something wrong of if this was already reported (I found some CIFS issues but not exactly this one).
I'm using x86-64 Arch Linux and LTS kernel (6.1.71 as I write this) and I noticed a regression that I could reproduce in other boxes with other architectures as well (aarch64 with 6.1.70).
# mount.cifs //server/share /mnt # mount //server/share on /mnt type cifs (rw,relatime,vers=3.1.1...) # cd /mnt # df . df: .: Resource temporarily unavailable # ls -al ls: .: Resource temporarily unavailable ls: file1: Resource temporarily unavailable ls: file2: Resource temporarily unavailable [...then ls shows the listing...]
If I use strace with df, the problem is: statfs(".", 0x.....) = -1 EAGAIN (Resource temporarily unavailable)
And with ls: listxattr(".", 0x..., 152): -1 EAGAIN (Resource temporarily unavailable) listxattr("file1", ..., 152): -1 EAGAIN (same as above) ...
Initially I thought the problem was with the Samba server and/or the client mount flags, but I've spent a day trying a *lot* of different combinations and nothing worked. This happens with any share that I try, and I've tried mounting shares from multiple Linux boxes running different Samba and kernel versions.
Then I tried changing kernel versions at my client box. I booted latest 6.6.9 and the problem simply disappeared. My Debian server with 6.5.11 also doesn't have it. I then started a VM and tried a "bisection" of 6.1.x versions, leading to kernel 6.1.70 when this started to happen. 6.1.69 and older look fine.
I hope that this is enough information to reproduce this issue. I will be glad to provide more info if necessary.
// Leonardo.
On Mon, Jan 08, 2024 at 12:18:26PM +0100, Jan Čermák wrote:
Hi,
I confirm Leonardo's findings about 6.1.70 introducing this regression, this issue manifested in Home Assistant OS [1] which was recently bumped to that version. I bisected the issue between 6.1.69 and 6.1.70 which pointed me to this bad commit:
commit bef4315f19ba6f434054f58b958c0cf058c7a43f (refs/bisect/bad) Author: Paulo Alcantara pc@manguebit.com Date: Wed Dec 13 12:25:57 2023 -0300
smb: client: fix OOB in SMB2_query_info_init() commit 33eae65c6f49770fec7a662935d4eb4a6406d24b upstream. A small CIFS buffer (448 bytes) isn't big enough to hold SMB2_QUERY_INFO request along with user's input data from CIFS_QUERY_INFO ioctl. That is, if the user passed an input buffer > 344 bytes, the client will memcpy() off the end of @req->Buffer in SMB2_query_info_init() thus causing the following KASAN splat:
(snip...)
Reverting this change on 6.1.y makes the error go away.
That's interesting, there's a different cifs report that says a different commit was the issue: https://lore.kernel.org/r/ZZhrpNJ3zxMR8wcU@eldamar.lan
is that the same as this one?
thanks,
greg k-h
Hi Greg
On 08. 01. 24 15:13, Greg KH wrote:
That's interesting, there's a different cifs report that says a different commit was the issue: https://lore.kernel.org/r/ZZhrpNJ3zxMR8wcU@eldamar.lan
is that the same as this one?
It seems to be a different issue. The one reported here by Leonardo doesn't trigger NULL pointer dereference and seems to be related to stat calls only, for which the CIFS client code in kernel just returns EAGAIN every time. The only related kernel buffer logs (example taken from the GH issue linked in my previous message) are these:
Jan 05 16:50:27 ha-ct kernel: CIFS: VFS: reconnect tcon failed rc = -11 Jan 05 16:50:30 ha-ct kernel: CIFS: VFS: \192.168.98.2 Send error in SessSetup = -11
If I understand it correctly, the issue you linked has both a different trigger and outcome.
Cheers, Jan
On 2024-01-08 15:13, Greg KH wrote:
That's interesting, there's a different cifs report that says a different commit was the issue: https://lore.kernel.org/r/ZZhrpNJ3zxMR8wcU@eldamar.lan
is that the same as this one?
It looks like a different issue. The linked report claims that the problem was introduced in 6.1.69 by a different commit, but both Jan Čermák and I don't experience anything wrong with 6.1.69. Jan Čermák found commit bef4315f19ba6f434054f58b958c0cf058c7a43f via bisection and compiled a kernel that reverts it, and the problem stopped manifesting.
Hi Jan,
Thanks for the report.
So this bug is related to an off-by-one in smb2_set_next_command() when the client attempts to pad SMB2_QUERY_INFO request -- since it isn't 8 byte aligned -- even though smb2_query_info_compound() doesn't provide an extra iov for such padding.
v6.1.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does
+ if (unlikely(check_add_overflow(input_len, sizeof(*req), &len) || + len > CIFSMaxBufSize)) + return -EINVAL; +
so sizeof(*req) will wrongly include the extra byte from smb2_query_info_req::Buffer making @len unaligned and therefore causing OOB in smb2_set_next_command().
A simple fix for that would be
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 05ff8a457a3d..aed5067661de 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -3556,7 +3556,7 @@ SMB2_query_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server, iov[0].iov_base = (char *)req; /* 1 for Buffer */ - iov[0].iov_len = len; + iov[0].iov_len = len - 1; return 0; }
On Mon, Jan 08, 2024 at 11:52:45AM -0300, Paulo Alcantara wrote:
Hi Jan,
Thanks for the report.
So this bug is related to an off-by-one in smb2_set_next_command() when the client attempts to pad SMB2_QUERY_INFO request -- since it isn't 8 byte aligned -- even though smb2_query_info_compound() doesn't provide an extra iov for such padding.
v6.1.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does
- if (unlikely(check_add_overflow(input_len, sizeof(*req), &len) ||
len > CIFSMaxBufSize))
return -EINVAL;
so sizeof(*req) will wrongly include the extra byte from smb2_query_info_req::Buffer making @len unaligned and therefore causing OOB in smb2_set_next_command().
A simple fix for that would be
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 05ff8a457a3d..aed5067661de 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -3556,7 +3556,7 @@ SMB2_query_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server, iov[0].iov_base = (char *)req; /* 1 for Buffer */
- iov[0].iov_len = len;
- iov[0].iov_len = len - 1; return 0;
}
Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to resolve this?
I've queued it up now.
thanks,
greg k-h
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to resolve this?
Yep, this is the right way to go.
I've queued it up now.
Thanks!
Hi Paulo, hi Greg,
Note this is about the 5.10.y backports of the cifs issue, were system calls fail with "Resource temporarily unavailable".
On Mon, Jan 08, 2024 at 12:58:49PM -0300, Paulo Alcantara wrote:
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to resolve this?
Yep, this is the right way to go.
I've queued it up now.
Thanks!
Is the underlying issue by picking the three commits:
3080ea5553cc ("stddef: Introduce DECLARE_FLEX_ARRAY() helper") eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the last commit in linux-stable-rc for 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
really fixing the issue?
Since we need to release a new update in Debian, I picked those three for testing on top of the 5.10.209-1 and while testing explicitly a cifs mount, I still get:
statfs(".", 0x7ffd809d5a70) = -1 EAGAIN (Resource temporarily unavailable)
The same happens if I build https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c... (knowing that it is not yet ready for review).
I'm slight confused as a280ecca48be ("cifs: fix off-by-one in SMB2_query_info_init()") says in the commit message:
[...] v5.10.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does [...]
and in meanwhile though the eb3e28c1e89b was picked (in a backported version). As 6.1.75-rc2 itself does not show the same problem, might there be a prerequisite missing in the backports for 5.10.y or a backport being wrong?
Regards, Salvatore
Hi Paulo, hi Greg,
On Tue, Jan 30, 2024 at 11:43:52PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
Note this is about the 5.10.y backports of the cifs issue, were system calls fail with "Resource temporarily unavailable".
On Mon, Jan 08, 2024 at 12:58:49PM -0300, Paulo Alcantara wrote:
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to resolve this?
Yep, this is the right way to go.
I've queued it up now.
Thanks!
Is the underlying issue by picking the three commits:
3080ea5553cc ("stddef: Introduce DECLARE_FLEX_ARRAY() helper") eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the last commit in linux-stable-rc for 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
really fixing the issue?
Since we need to release a new update in Debian, I picked those three for testing on top of the 5.10.209-1 and while testing explicitly a cifs mount, I still get:
statfs(".", 0x7ffd809d5a70) = -1 EAGAIN (Resource temporarily unavailable)
The same happens if I build https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c... (knowing that it is not yet ready for review).
I'm slight confused as a280ecca48be ("cifs: fix off-by-one in SMB2_query_info_init()") says in the commit message:
[...] v5.10.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does [...]
and in meanwhile though the eb3e28c1e89b was picked (in a backported version). As 6.1.75-rc2 itself does not show the same problem, might there be a prerequisite missing in the backports for 5.10.y or a backport being wrong?
The problem seems to be that we are picking the backport for eb3e28c1e89b, but then still applying
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
which was made for the case in 5.10.y where eb3e28c1e89b is not present.
I reverted a280ecca48beb40ca6c0fc20dd5 and now:
statfs(".", {f_type=SMB2_MAGIC_NUMBER, f_bsize=4096, f_blocks=2189197, f_bfree=593878, f_bavail=593878, f_files=0, f_ffree=0, f_fsid={val=[2004816114, 0]}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_RELATIME}) = 0
Regards, Salvatore
On Tue, Jan 30, 2024 at 11:49:23PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
On Tue, Jan 30, 2024 at 11:43:52PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
Note this is about the 5.10.y backports of the cifs issue, were system calls fail with "Resource temporarily unavailable".
On Mon, Jan 08, 2024 at 12:58:49PM -0300, Paulo Alcantara wrote:
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to resolve this?
Yep, this is the right way to go.
I've queued it up now.
Thanks!
Is the underlying issue by picking the three commits:
3080ea5553cc ("stddef: Introduce DECLARE_FLEX_ARRAY() helper") eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the last commit in linux-stable-rc for 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
really fixing the issue?
Since we need to release a new update in Debian, I picked those three for testing on top of the 5.10.209-1 and while testing explicitly a cifs mount, I still get:
statfs(".", 0x7ffd809d5a70) = -1 EAGAIN (Resource temporarily unavailable)
The same happens if I build https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c... (knowing that it is not yet ready for review).
I'm slight confused as a280ecca48be ("cifs: fix off-by-one in SMB2_query_info_init()") says in the commit message:
[...] v5.10.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does [...]
and in meanwhile though the eb3e28c1e89b was picked (in a backported version). As 6.1.75-rc2 itself does not show the same problem, might there be a prerequisite missing in the backports for 5.10.y or a backport being wrong?
The problem seems to be that we are picking the backport for eb3e28c1e89b, but then still applying
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
which was made for the case in 5.10.y where eb3e28c1e89b is not present.
I reverted a280ecca48beb40ca6c0fc20dd5 and now:
statfs(".", {f_type=SMB2_MAGIC_NUMBER, f_bsize=4096, f_blocks=2189197, f_bfree=593878, f_bavail=593878, f_files=0, f_ffree=0, f_fsid={val=[2004816114, 0]}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_RELATIME}) = 0
So this works? Would that just be easier to do overall? I feel like that might be best here.
Again, a set of simple "do this and this and this" would be nice to have, as there are too many threads here, some incomplete and missing commits on my end.
confused,
greg k-h
Hi Greg,
On Tue, Feb 20, 2024 at 09:27:49PM +0100, Greg Kroah-Hartman wrote:
On Tue, Jan 30, 2024 at 11:49:23PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
On Tue, Jan 30, 2024 at 11:43:52PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
Note this is about the 5.10.y backports of the cifs issue, were system calls fail with "Resource temporarily unavailable".
On Mon, Jan 08, 2024 at 12:58:49PM -0300, Paulo Alcantara wrote:
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to resolve this?
Yep, this is the right way to go.
I've queued it up now.
Thanks!
Is the underlying issue by picking the three commits:
3080ea5553cc ("stddef: Introduce DECLARE_FLEX_ARRAY() helper") eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the last commit in linux-stable-rc for 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
really fixing the issue?
Since we need to release a new update in Debian, I picked those three for testing on top of the 5.10.209-1 and while testing explicitly a cifs mount, I still get:
statfs(".", 0x7ffd809d5a70) = -1 EAGAIN (Resource temporarily unavailable)
The same happens if I build https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c... (knowing that it is not yet ready for review).
I'm slight confused as a280ecca48be ("cifs: fix off-by-one in SMB2_query_info_init()") says in the commit message:
[...] v5.10.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does [...]
and in meanwhile though the eb3e28c1e89b was picked (in a backported version). As 6.1.75-rc2 itself does not show the same problem, might there be a prerequisite missing in the backports for 5.10.y or a backport being wrong?
The problem seems to be that we are picking the backport for eb3e28c1e89b, but then still applying
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
which was made for the case in 5.10.y where eb3e28c1e89b is not present.
I reverted a280ecca48beb40ca6c0fc20dd5 and now:
statfs(".", {f_type=SMB2_MAGIC_NUMBER, f_bsize=4096, f_blocks=2189197, f_bfree=593878, f_bavail=593878, f_files=0, f_ffree=0, f_fsid={val=[2004816114, 0]}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_RELATIME}) = 0
So this works? Would that just be easier to do overall? I feel like that might be best here.
Again, a set of simple "do this and this and this" would be nice to have, as there are too many threads here, some incomplete and missing commits on my end.
confused,
It is quite chaotic, since I believe multiple people worked on trying to resolve the issue, and then for the 5.10.y and 5.15.y branches different initial commits were applied.
For 5.10.y it's the case: Keep the backport of eb3e28c1e89b and drop a280ecca48be (as it is not true that v5.10.y does not have eb3e28c1e89b, as it is actually in the current 5.10.y queue).
Paulo can you please give Greg an authoratitative set of commits to keep/apply in the 5.10.y and 5.15.y series.
Regards, Salvatore
On Tue, Feb 20, 2024 at 10:25:16PM +0100, Salvatore Bonaccorso wrote:
Hi Greg,
On Tue, Feb 20, 2024 at 09:27:49PM +0100, Greg Kroah-Hartman wrote:
On Tue, Jan 30, 2024 at 11:49:23PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
On Tue, Jan 30, 2024 at 11:43:52PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
Note this is about the 5.10.y backports of the cifs issue, were system calls fail with "Resource temporarily unavailable".
On Mon, Jan 08, 2024 at 12:58:49PM -0300, Paulo Alcantara wrote:
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to resolve this?
Yep, this is the right way to go.
I've queued it up now.
Thanks!
Is the underlying issue by picking the three commits:
3080ea5553cc ("stddef: Introduce DECLARE_FLEX_ARRAY() helper") eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the last commit in linux-stable-rc for 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
really fixing the issue?
Since we need to release a new update in Debian, I picked those three for testing on top of the 5.10.209-1 and while testing explicitly a cifs mount, I still get:
statfs(".", 0x7ffd809d5a70) = -1 EAGAIN (Resource temporarily unavailable)
The same happens if I build https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c... (knowing that it is not yet ready for review).
I'm slight confused as a280ecca48be ("cifs: fix off-by-one in SMB2_query_info_init()") says in the commit message:
[...] v5.10.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does [...]
and in meanwhile though the eb3e28c1e89b was picked (in a backported version). As 6.1.75-rc2 itself does not show the same problem, might there be a prerequisite missing in the backports for 5.10.y or a backport being wrong?
The problem seems to be that we are picking the backport for eb3e28c1e89b, but then still applying
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
which was made for the case in 5.10.y where eb3e28c1e89b is not present.
I reverted a280ecca48beb40ca6c0fc20dd5 and now:
statfs(".", {f_type=SMB2_MAGIC_NUMBER, f_bsize=4096, f_blocks=2189197, f_bfree=593878, f_bavail=593878, f_files=0, f_ffree=0, f_fsid={val=[2004816114, 0]}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_RELATIME}) = 0
So this works? Would that just be easier to do overall? I feel like that might be best here.
Again, a set of simple "do this and this and this" would be nice to have, as there are too many threads here, some incomplete and missing commits on my end.
confused,
It is quite chaotic, since I believe multiple people worked on trying to resolve the issue, and then for the 5.10.y and 5.15.y branches different initial commits were applied.
For 5.10.y it's the case: Keep the backport of eb3e28c1e89b and drop a280ecca48be (as it is not true that v5.10.y does not have eb3e28c1e89b, as it is actually in the current 5.10.y queue).
I think we are good now.
Paulo can you please give Greg an authoratitative set of commits to keep/apply in the 5.10.y and 5.15.y series.
Yes, anything I missed?
thanks,
greg k-h
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
On Tue, Feb 20, 2024 at 10:25:16PM +0100, Salvatore Bonaccorso wrote:
Hi Greg,
On Tue, Feb 20, 2024 at 09:27:49PM +0100, Greg Kroah-Hartman wrote:
On Tue, Jan 30, 2024 at 11:49:23PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
On Tue, Jan 30, 2024 at 11:43:52PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
Note this is about the 5.10.y backports of the cifs issue, were system calls fail with "Resource temporarily unavailable".
On Mon, Jan 08, 2024 at 12:58:49PM -0300, Paulo Alcantara wrote:
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
> Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element > arrays with flex-arrays") to resolve this?
Yep, this is the right way to go.
> I've queued it up now.
Thanks!
Is the underlying issue by picking the three commits:
3080ea5553cc ("stddef: Introduce DECLARE_FLEX_ARRAY() helper") eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the last commit in linux-stable-rc for 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
really fixing the issue?
Since we need to release a new update in Debian, I picked those three for testing on top of the 5.10.209-1 and while testing explicitly a cifs mount, I still get:
statfs(".", 0x7ffd809d5a70) = -1 EAGAIN (Resource temporarily unavailable)
The same happens if I build https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c... (knowing that it is not yet ready for review).
I'm slight confused as a280ecca48be ("cifs: fix off-by-one in SMB2_query_info_init()") says in the commit message:
[...] v5.10.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does [...]
and in meanwhile though the eb3e28c1e89b was picked (in a backported version). As 6.1.75-rc2 itself does not show the same problem, might there be a prerequisite missing in the backports for 5.10.y or a backport being wrong?
The problem seems to be that we are picking the backport for eb3e28c1e89b, but then still applying
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
which was made for the case in 5.10.y where eb3e28c1e89b is not present.
I reverted a280ecca48beb40ca6c0fc20dd5 and now:
statfs(".", {f_type=SMB2_MAGIC_NUMBER, f_bsize=4096, f_blocks=2189197, f_bfree=593878, f_bavail=593878, f_files=0, f_ffree=0, f_fsid={val=[2004816114, 0]}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_RELATIME}) = 0
So this works? Would that just be easier to do overall? I feel like that might be best here.
Again, a set of simple "do this and this and this" would be nice to have, as there are too many threads here, some incomplete and missing commits on my end.
confused,
It is quite chaotic, since I believe multiple people worked on trying to resolve the issue, and then for the 5.10.y and 5.15.y branches different initial commits were applied.
For 5.10.y it's the case: Keep the backport of eb3e28c1e89b and drop a280ecca48be (as it is not true that v5.10.y does not have eb3e28c1e89b, as it is actually in the current 5.10.y queue).
I think we are good now.
Paulo can you please give Greg an authoratitative set of commits to keep/apply in the 5.10.y and 5.15.y series.
Yes, anything I missed?
The one-liner fix (a280ecca48be) provided by Harshit was only required if not backporting eb3e28c1e89b. As both 5.10.y and 5.15.y now have eb3e28c1e89b queued up, LGTM.
Salvatore, please let us know if you can still hit the issue with eb3e28c1e89b applied.
Hi Paulo,
On Thu, Feb 22, 2024 at 08:00:58PM -0300, Paulo Alcantara wrote:
Greg Kroah-Hartman gregkh@linuxfoundation.org writes:
On Tue, Feb 20, 2024 at 10:25:16PM +0100, Salvatore Bonaccorso wrote:
Hi Greg,
On Tue, Feb 20, 2024 at 09:27:49PM +0100, Greg Kroah-Hartman wrote:
On Tue, Jan 30, 2024 at 11:49:23PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
On Tue, Jan 30, 2024 at 11:43:52PM +0100, Salvatore Bonaccorso wrote:
Hi Paulo, hi Greg,
Note this is about the 5.10.y backports of the cifs issue, were system calls fail with "Resource temporarily unavailable".
On Mon, Jan 08, 2024 at 12:58:49PM -0300, Paulo Alcantara wrote: > Greg Kroah-Hartman gregkh@linuxfoundation.org writes: > > > Why can't we just include eb3e28c1e89b ("smb3: Replace smb2pdu 1-element > > arrays with flex-arrays") to resolve this? > > Yep, this is the right way to go. > > > I've queued it up now. > > Thanks!
Is the underlying issue by picking the three commits:
3080ea5553cc ("stddef: Introduce DECLARE_FLEX_ARRAY() helper") eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the last commit in linux-stable-rc for 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
really fixing the issue?
Since we need to release a new update in Debian, I picked those three for testing on top of the 5.10.209-1 and while testing explicitly a cifs mount, I still get:
statfs(".", 0x7ffd809d5a70) = -1 EAGAIN (Resource temporarily unavailable)
The same happens if I build https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c... (knowing that it is not yet ready for review).
I'm slight confused as a280ecca48be ("cifs: fix off-by-one in SMB2_query_info_init()") says in the commit message:
[...] v5.10.y doesn't have
eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
and the commit does [...]
and in meanwhile though the eb3e28c1e89b was picked (in a backported version). As 6.1.75-rc2 itself does not show the same problem, might there be a prerequisite missing in the backports for 5.10.y or a backport being wrong?
The problem seems to be that we are picking the backport for eb3e28c1e89b, but then still applying
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/c...
which was made for the case in 5.10.y where eb3e28c1e89b is not present.
I reverted a280ecca48beb40ca6c0fc20dd5 and now:
statfs(".", {f_type=SMB2_MAGIC_NUMBER, f_bsize=4096, f_blocks=2189197, f_bfree=593878, f_bavail=593878, f_files=0, f_ffree=0, f_fsid={val=[2004816114, 0]}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_RELATIME}) = 0
So this works? Would that just be easier to do overall? I feel like that might be best here.
Again, a set of simple "do this and this and this" would be nice to have, as there are too many threads here, some incomplete and missing commits on my end.
confused,
It is quite chaotic, since I believe multiple people worked on trying to resolve the issue, and then for the 5.10.y and 5.15.y branches different initial commits were applied.
For 5.10.y it's the case: Keep the backport of eb3e28c1e89b and drop a280ecca48be (as it is not true that v5.10.y does not have eb3e28c1e89b, as it is actually in the current 5.10.y queue).
I think we are good now.
Paulo can you please give Greg an authoratitative set of commits to keep/apply in the 5.10.y and 5.15.y series.
Yes, anything I missed?
The one-liner fix (a280ecca48be) provided by Harshit was only required if not backporting eb3e28c1e89b. As both 5.10.y and 5.15.y now have eb3e28c1e89b queued up, LGTM.
Salvatore, please let us know if you can still hit the issue with eb3e28c1e89b applied.
Correct, I cannot reproduce anymore the issue with 5.10.210-rc1.
Regards, Salvatore
linux-stable-mirror@lists.linaro.org