| Message ID | 20220331215541.883746-1-jakobkoschel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] cifs: replace unnecessary use of list iterator variable with head | expand |
It looks like this no longer applies cleanly. Can you recheck
fs/cifs/smb2pdu.c (function smb2_reconnect_server) and see if it
applies now that it has changed e.g.
/* allocate a dummy tcon struct used for reconnect */
tcon = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
if (!tcon) {
resched = true;
list_for_each_entry_safe(ses, ses2, &tmp_ses_list, rlist) {
list_del_init(&ses->rlist);
cifs_put_smb_ses(ses);
}
goto done;
}
You had this:
--- fs/cifs/smb2pdu.c
+++ fs/cifs/smb2pdu.c
@@ -3858,7 +3858,7 @@ void smb2_reconnect_server(struct work_struct *work)
tcon = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
if (!tcon) {
resched = true;
- list_del_init(&ses->rlist);
+ list_del_init(&pserver->smb_ses_list);
cifs_put_smb_ses(ses);
goto done;
}
On Fri, Apr 1, 2022 at 2:23 AM Jakob Koschel <jakobkoschel@gmail.com> wrote:
>
> When list_for_each_entry() completes the iteration over the whole list
> without breaking the loop, the iterator variable will *always* be a
> bogus pointer computed based on the head element.
>
> To avoid type confusion use the actual list head directly instead of
> the last iterator value.
>
> Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com>
> ---
> fs/cifs/smb2pdu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 7e7909b1ae11..4ac86b77a7c9 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -3858,7 +3858,7 @@ void smb2_reconnect_server(struct work_struct *work)
> tcon = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL);
> if (!tcon) {
> resched = true;
> - list_del_init(&ses->rlist);
> + list_del_init(&pserver->smb_ses_list);
> cifs_put_smb_ses(ses);
> goto done;
> }
>
> base-commit: f82da161ea75dc4db21b2499e4b1facd36dab275
> --
> 2.25.1
>
Hey Steve, > On 2. Apr 2022, at 00:44, Steve French <smfrench@gmail.com> wrote: > > It looks like this no longer applies cleanly. Can you recheck > fs/cifs/smb2pdu.c (function smb2_reconnect_server) and see if it > applies now that it has changed e.g. looks like this issue was already fixed in the meantime. Feel free to ignore PATCH 1/2. > > > /* allocate a dummy tcon struct used for reconnect */ > tcon = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL); > if (!tcon) { > resched = true; > list_for_each_entry_safe(ses, ses2, &tmp_ses_list, rlist) { > list_del_init(&ses->rlist); > cifs_put_smb_ses(ses); > } > goto done; > } > > You had this: > > --- fs/cifs/smb2pdu.c > +++ fs/cifs/smb2pdu.c > @@ -3858,7 +3858,7 @@ void smb2_reconnect_server(struct work_struct *work) > tcon = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL); > if (!tcon) { > resched = true; > - list_del_init(&ses->rlist); > + list_del_init(&pserver->smb_ses_list); > cifs_put_smb_ses(ses); > goto done; > } > > On Fri, Apr 1, 2022 at 2:23 AM Jakob Koschel <jakobkoschel@gmail.com> wrote: >> >> When list_for_each_entry() completes the iteration over the whole list >> without breaking the loop, the iterator variable will *always* be a >> bogus pointer computed based on the head element. >> >> To avoid type confusion use the actual list head directly instead of >> the last iterator value. >> >> Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com> >> --- >> fs/cifs/smb2pdu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c >> index 7e7909b1ae11..4ac86b77a7c9 100644 >> --- a/fs/cifs/smb2pdu.c >> +++ b/fs/cifs/smb2pdu.c >> @@ -3858,7 +3858,7 @@ void smb2_reconnect_server(struct work_struct *work) >> tcon = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL); >> if (!tcon) { >> resched = true; >> - list_del_init(&ses->rlist); >> + list_del_init(&pserver->smb_ses_list); >> cifs_put_smb_ses(ses); >> goto done; >> } >> >> base-commit: f82da161ea75dc4db21b2499e4b1facd36dab275 >> -- >> 2.25.1 >> > > > -- > Thanks, > > Steve
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 7e7909b1ae11..4ac86b77a7c9 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -3858,7 +3858,7 @@ void smb2_reconnect_server(struct work_struct *work) tcon = kzalloc(sizeof(struct cifs_tcon), GFP_KERNEL); if (!tcon) { resched = true; - list_del_init(&ses->rlist); + list_del_init(&pserver->smb_ses_list); cifs_put_smb_ses(ses); goto done; }
When list_for_each_entry() completes the iteration over the whole list without breaking the loop, the iterator variable will *always* be a bogus pointer computed based on the head element. To avoid type confusion use the actual list head directly instead of the last iterator value. Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com> --- fs/cifs/smb2pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: f82da161ea75dc4db21b2499e4b1facd36dab275