On Mon, 2024-05-06 at 16:52 -0400, cel@xxxxxxxxxx wrote:
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> Commit 5866efa8cbfb ("SUNRPC: Fix svcauth_gss_proxy_init()") from Oct
> 24, 2019 (linux-next), leads to the following Smatch static checker
> warning:
>
> net/sunrpc/auth_gss/svcauth_gss.c:1039
> gss_free_in_token_pages()
> warn: iterator 'i' not incremented
>
> net/sunrpc/auth_gss/svcauth_gss.c
> 1034 static void gss_free_in_token_pages(struct gssp_in_token
> *in_token)
> 1035 {
> 1036 u32 inlen;
> 1037 int i;
> 1038
> --> 1039 i = 0;
> 1040 inlen = in_token->page_len;
> 1041 while (inlen) {
> 1042 if (in_token->pages[i])
> 1043 put_page(in_token->pages[i]);
> ^
> This puts page zero over and over.
>
> 1044 inlen -= inlen > PAGE_SIZE ? PAGE_SIZE :
> inlen;
> 1045 }
> 1046
> 1047 kfree(in_token->pages);
> 1048 in_token->pages = NULL;
> 1049 }
>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Fixes: 5866efa8cbfb ("SUNRPC: Fix svcauth_gss_proxy_init()")
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> net/sunrpc/auth_gss/svcauth_gss.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c
> b/net/sunrpc/auth_gss/svcauth_gss.c
> index 24de94184700..bdd8273d74d3 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -1040,7 +1040,7 @@ static void gss_free_in_token_pages(struct
> gssp_in_token *in_token)
> inlen = in_token->page_len;
> while (inlen) {
> if (in_token->pages[i])
> - put_page(in_token->pages[i]);
> + put_page(in_token->pages[i++]);
Wouldn't it be both more efficient and transparent just to break out of
the loop once you hit a NULL page? You already know the remainder of
the array will also be NULL.
> inlen -= inlen > PAGE_SIZE ? PAGE_SIZE : inlen;
> }
>
>
> base-commit: 939cb14d51a150e3c12ef7a8ce0ba04ce6131bd2
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
