On Fri, Dec 29, 2023 at 4:46 PM <nspmangalore@xxxxxxxxx> wrote:
>
> From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
>
> parse_server_interfaces should be in complete charge of maintaining
> the iface_list linked list. Today, iface entries are removed
> from the list only when the last refcount is dropped.
> i.e. in release_iface. However, this can result in undercounting
> of refcount if the server stops advertising interfaces (which
> Azure SMB server does).
>
> This change puts parse_server_interfaces in full charge of
> maintaining the iface_list. So if an empty list is returned
> by the server, the entries in the list will immediately be
> removed. This way, a following call to the same function will
> not find entries in the list.
>
> Fixes: aa45dadd34e4 ("cifs: change iface_list from array to sorted linked list")
> Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
> ---
> fs/smb/client/cifsglob.h | 1 -
> fs/smb/client/smb2ops.c | 27 +++++++++++++++++----------
> 2 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index ba80c854c9ca..f840756e0169 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1014,7 +1014,6 @@ release_iface(struct kref *ref)
> struct cifs_server_iface *iface = container_of(ref,
> struct cifs_server_iface,
> refcount);
> - list_del_init(&iface->iface_head);
> kfree(iface);
> }
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 104c58df0368..b813485c0e86 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -595,16 +595,12 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
> }
>
> /*
> - * Go through iface_list and do kref_put to remove
> - * any unused ifaces. ifaces in use will be removed
> - * when the last user calls a kref_put on it
> + * Go through iface_list and mark them as inactive
> */
> list_for_each_entry_safe(iface, niface, &ses->iface_list,
> - iface_head) {
> + iface_head)
> iface->is_active = 0;
> - kref_put(&iface->refcount, release_iface);
> - ses->iface_count--;
> - }
> +
> spin_unlock(&ses->iface_lock);
>
> /*
> @@ -678,10 +674,7 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
> iface_head) {
> ret = iface_cmp(iface, &tmp_iface);
> if (!ret) {
> - /* just get a ref so that it doesn't get picked/freed */
> iface->is_active = 1;
> - kref_get(&iface->refcount);
> - ses->iface_count++;
> spin_unlock(&ses->iface_lock);
> goto next_iface;
> } else if (ret < 0) {
> @@ -748,6 +741,20 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
> }
>
> out:
> + /*
> + * Go through the list again and put the inactive entries
> + */
> + spin_lock(&ses->iface_lock);
> + list_for_each_entry_safe(iface, niface, &ses->iface_list,
> + iface_head) {
> + if (!iface->is_active) {
> + list_del(&iface->iface_head);
> + kref_put(&iface->refcount, release_iface);
> + ses->iface_count--;
> + }
> + }
> + spin_unlock(&ses->iface_lock);
> +
> return rc;
> }
>
> --
> 2.34.1
>
Hi Steve..
This one should also be marked for stable. I missed tagging it before I sent.
--
Regards,
Shyam
