Re: [PATCH v2] util/memfd: allow allocating 0 bytes

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, May 06, 2025 at 07:41:32PM +0300, Elisha Hollander wrote:
> Gave an example for a case where QEMU would try to allocate 0 bytes thus
> fail here in the original version of the patch.
> 
> > As I mentioned earlier, let's say you don't initialize the vertical
> display end registers, and set the minimum scanline register, the emulation
> will then have to allocate some display buffer, but because the vertical
> display end is initilized as 0 the buffer will be empty and the program
> break.

Isn't this an invalid hardware configuration that should be detected
in the emulation code, and either force the display end to a minimum
value, or trigger an assert ?

Patching a bug in a specific HW impl, by changing the qemu_memfd_alloc
code feels like it is probably the wrong place to address this.

> 
> I have no idea as for why my emails are getting messed up... :/
> 
> Have to go now, will try and send it again tomorrow probably...
> 
> On Tue, May 6, 2025, 19:37 Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> > On Tue, May 06, 2025 at 07:17:25PM +0300, Elisha Hollander wrote:
> > > Sorry for former patch something is messed up with my email.
> >
> > The commit message needs to explain what problem is being solved by
> > making this change as allowing 0 bytes looks dubious on the surface.
> >
> > >
> > > Signed-off-by: donno2048 <just4now666666@gmail.com>
> > > ---
> > >  util/memfd.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/util/memfd.c b/util/memfd.c
> > > index 8a2e906..e96e5af 100644
> > > --- a/util/memfd.c
> > > +++ b/util/memfd.c
> > > @@ -108,7 +108,7 @@ err:
> > >  void *qemu_memfd_alloc(const char *name, size_t size, unsigned int
> > seals,
> > >                         int *fd, Error **errp)
> > >  {
> > > - void *ptr;
> > > + void *ptr = NULL;
> > >      int mfd = qemu_memfd_create(name, size, false, 0, seals, NULL);
> > >
> > >      /* some systems have memfd without sealing */
> > > @@ -131,9 +131,11 @@ void *qemu_memfd_alloc(const char *name, size_t
> > size,
> > > unsigned int seals,
> > >          }
> > >      }
> > >
> > > - ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
> > > - if (ptr == MAP_FAILED) {
> > > - goto err;
> > > + if (size != 0) {
> > > + ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
> > > + if (ptr == MAP_FAILED) {
> > > + goto err;
> > > + }
> > >      }
> >
> > This patch is mangled.
> >
> >
> > With regards,
> > Daniel
> > --
> > |: https://berrange.com      -o-
> > https://www.flickr.com/photos/dberrange :|
> > |: https://libvirt.org         -o-
> > https://fstop138.berrange.com :|
> > |: https://entangle-photo.org    -o-
> > https://www.instagram.com/dberrange :|
> >
> >

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|