On Fri, Apr 05, 2019 at 11:34:40AM +1100, NeilBrown wrote:
> On Fri, Apr 05 2019, NeilBrown wrote:
> >
> > Unfortunately I was wrong. See below.
> >
>
> Arrghh.. Ignore that patch. Use this one.
Took me a moment to spot the difference. OK! Queuing that up for
5.1....
--b.
>
> NeilBrown
>
> From b7a8dced1a34869710a5d68a64e8bd37bf2ab426 Mon Sep 17 00:00:00 2001
> From: NeilBrown <neilb@xxxxxxxx>
> Date: Fri, 5 Apr 2019 09:04:41 +1100
> Subject: [PATCH] sunrpc: don't mark uninitialised items as VALID.
>
> A recent commit added a call to cache_fresh_locked()
> when an expired item was found.
> The call sets the CACHE_VALID flag, so it is important
> that the item actually is valid.
> There are two ways it could be valid:
> 1/ If ->update has been called to fill in relevant content
> 2/ if CACHE_NEGATIVE is set, to say that content doesn't exist.
>
> An expired item that is waiting for an update will be neither.
> Setting CACHE_VALID will mean that a subsequent call to cache_put()
> will be likely to dereference uninitialised pointers.
>
> So we must make sure the item is valid, and we already have code to do
> that in try_to_negate_entry(). This takes the hash lock and so cannot
> be used directly, so take out the two lines that we need and use them.
>
> Now cache_fresh_locked() is certain to be called only on
> a valid item.
>
> Cc: stable@xxxxxxxxxx # 2.6.35
> Fixes: 4ecd55ea0742 ("sunrpc: fix cache_head leak due to queued request")
> Signed-off-by: NeilBrown <neilb@xxxxxxxx>
> ---
> net/sunrpc/cache.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 12bb23b8e0c5..261131dfa1f1 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -54,6 +54,7 @@ static void cache_init(struct cache_head *h, struct cache_detail *detail)
> h->last_refresh = now;
> }
>
> +static inline int cache_is_valid(struct cache_head *h);
> static void cache_fresh_locked(struct cache_head *head, time_t expiry,
> struct cache_detail *detail);
> static void cache_fresh_unlocked(struct cache_head *head,
> @@ -105,6 +106,8 @@ static struct cache_head *sunrpc_cache_add_entry(struct cache_detail *detail,
> if (cache_is_expired(detail, tmp)) {
> hlist_del_init_rcu(&tmp->cache_list);
> detail->entries --;
> + if (cache_is_valid(tmp) == -EAGAIN)
> + set_bit(CACHE_NEGATIVE, &tmp->flags);
> cache_fresh_locked(tmp, 0, detail);
> freeme = tmp;
> break;
> --
> 2.14.0.rc0.dirty
>
