Re: [PATCH 1/2] smb: client: fill cache fields after populating cache in copy_ref_data()

[Paulo Alcantara]

Fredric Cover <fredric.cover.lkernel@xxxxxxxxx> writes:

> In copy_ref_data(), struct cache_entry *ce has its fields populated
> at the beginning of the function. Later, if alloc_target fails with
> an ERR_PTR, free_tgts() is called on the cache, leaving the cache
> metadata populated without any targets. Critically, this extends
> ce->etime, making the cache appear valid for longer without any
> targets.
>
> Also, free_tgts() does not set ce->numtgts to zero. On error, when
> the cache is freed, ce->numtgts is not zeroed, and other cache users
> may attempt to access nonexistent entries.
>
> Update fields after copying targets to prevent partial-state updates.
> Set ce->numtgts to zero at the end of free_tgts().
>
> Signed-off-by: Fredric Cover <fredric.cover.lkernel@xxxxxxxxx>
> ---
>  fs/smb/client/dfs_cache.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
> index 86dba25b7a5a..b0388c460499 100644
> --- a/fs/smb/client/dfs_cache.c
> +++ b/fs/smb/client/dfs_cache.c
> @@ -123,6 +123,7 @@ static inline void free_tgts(struct cache_entry *ce)
>  		kfree(t);
>  	}
>  
> +	ce->numtgts = 0;

Since you're now setting ce->numtgts to 0 in free_tgts(), you could get
rid of the same setting in update_cache_entry_locked() after free_tgts()
is called.

The rest looks good.