Re: [PATCH v2 2/6] xen/livepatch: zero pointer to temporary load buffer

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Sep 25, 2024 at 10:33:39AM +0100, Andrew Cooper wrote:
> On 25/09/2024 9:42 am, Roger Pau Monne wrote:
> > The livepatch_elf_sec data field points to the temporary load buffer, it's 
> > the
> > load_addr field that points to the stable loaded section data.  Zero the 
> > data
> > field once load_addr is set, as it would otherwise become a dangling pointer
> > once the load buffer is freed.
> >
> > No functional change intended.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > Changes since v1:
> >  - New in this version.
> > ---
> >  xen/common/livepatch.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> > index df41dcce970a..87b3db03e26d 100644
> > --- a/xen/common/livepatch.c
> > +++ b/xen/common/livepatch.c
> > @@ -383,6 +383,9 @@ static int move_payload(struct payload *payload, struct 
> > livepatch_elf *elf)
> >              }
> >              else
> >                  memset(elf->sec[i].load_addr, 0, elf->sec[i].sec->sh_size);
> > +
> > +            /* Avoid leaking pointers to temporary load buffers. */
> > +            elf->sec[i].data = NULL;
> >          }
> >      }
> >  
> 
> Where is the data allocated and freed?
> 
> I don't see it being freed in this loop, so how is freed subsequently?

It's allocated and freed by livepatch_upload(), it's the raw_data
buffer that's allocated in the context of that function.

Thanks, Roger.