Re: [for-6.13 PATCH 01/19] nfs/localio: must clear res.replen in nfs_local_read_done

NeilBrown
2024-11-11T00:36:27Z

On Sat, 09 Nov 2024, Mike Snitzer wrote:
> From: NeilBrown <neilb@suse.de>
>
> Otherwise memory corruption can occur due to NFSv3 LOCALIO reads
> leaving garbage in res.replen:

I'm not comfortable with this patch. It doesn't tell us *why* there is
garbage in res.replen.
This is part of nfs_pgio_header and whenever that is allocated it
initialised to all zeros. So where does the garbage come from?

Answer: it comes from
hdr->res.verf = &hdr->verf;
in nfs_pgio_rpcsetup().

struct nfs_pgio_res contains a union. 'replen' is present for read.
'verf' is present for write (and there is other stuff).

so I think that init of res.verf should only happen for write.

I cannot see an easy way to do that. The best I can come up with is
to add a new pg_ioflags flag which says "this is a write", and only
initialise res.verf if that is set.

If we do stick with the current patch, I'd like a comment where we set
res.replen saying that it was corrupted when res.verf was initialised in
nfs_gpio_rpcsetup().

Or maybe move res.replen out of the union. There is a 4byte hole before
the union (on x86_64). It would be cleaner to move verf out, but that
is bigger....

Thanks,
NeilBrown