Re: [musl] [PATCH] fix: Assign default value to mntent when linebuf is too small

[Kaihang Zhang <kaihang.zhang@smartx.com>]

On Tue, Oct 12, 2021 at 8:59 PM Rich Felker <dalias@libc.org> wrote:
>
> On Tue, Oct 12, 2021 at 03:24:07AM +0000, (GalaxyMaster) wrote:
> > Kaihang,
> >
> > On Mon, Oct 11, 2021 at 10:36:43PM -0400, Kaihang Zhang wrote:
> > > Function getmntent_r in source misc/mntent.c will do what glibc users
> > > expect. The rest of the line will be discarded when can not be read
> > > into linebuf, and the fields of struct mntent will be assigned to empty
> > > string or zero when can not be found in linebuf, instead of setting
> > > errno to ERANGE and exiting.
> >
> > Although this patch is on a similar topic as mine (changing the behaviour of
> > get*ent() funnctions), I think the change you are describing is considerable.
> >
> > I would expect a function such as getmntent_r() which takes a user provided
> > buffer to fail and set ERANGE if the provided buffer is not enough to hold
> > the line.  This gives the developer an opportunity to recover, e.g. to
> > re-allocate a bigger buffer and try again.
> >
> > In your proposal, I see two issues:
> >
> > 1. There is no feedback to the developer, so they have no idea whether the
> >    information they've got from the function was truncated or not (and what
> >    good does a truncated mnt line bring?);
> > 2. There is no opportunity for the developer to realise a mistake they made
> >    by supplying too small buffer, hence there is no chance of recovering
> >    from it.
> >
> > It is just my opinion and I would love to see other comments, since I have
> > not stumbled upon your use case yet and am not authoritative on this topic.
>
> Yes. There are actually several conflicting proposed changes to this
> (frustratingly underspecified, and with bad behavior by glibc)
> function, and so far there's been little engagement between different
> people who want it changed/fixed to resolve the differences. I would
> really like to see from at least one party who wants it changed a
> summary of what the differences are between the different proposals,
> what musl is doing now, and what glibc is doing now, and
> justifications for why their preferred one is okay (including
> capability for applications to recover and not silently use wrong data
> -- "glibc lets them silently use wrong data" isn't a justification).
>
> Rich

Give an example to better describe it. The contents of /proc/mounts
are like below,
the field 'opts' of overlay can be even more longer.

> overlay / overlay rw,seclabel,relatime,lowerdir=/var/lib/docker/overlay2/l/6WSJ5KZAYZO55RQRWFQZNDGPWK:/var/lib/docker/overlay2/l/YS7ZKHUX3JB3F4SQIJYXXNIYTV:/var/lib/docker/overlay2/l/OWUPK4QNZP7ZOZKERGMTJQW55T:/var/lib/docker/overlay2/l/LL7NIEVB5NMPAEJK6SUCXJYRGK,upperdir=/var/lib/docker/overlay2/2012ae841481001d5eea099c6e84319f88c5079d44eb6a2402397858dd3d22e2/diff,workdir=/var/lib/docker/overlay2/2012ae841481001d5eea099c6e84319f88c5079d44eb6a2402397858dd3d22e2/work 0 0
> proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0
> cgroup /sys/fs/cgroup/systemd cgroup rw,seclabel,nosuid,nodev,noexec,relatime,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd 0 0

1. The behavior of getmntent_r in glibc.
 If the buffer is too samll for the line 'overlay', the line will be truncated.
 Glibc uses wrong data for this line. But i only care about cgroup info,
 and i am sure that the buffer capacity is sufficient for cgroup. The mntent
 for cgroup is exactly what i want. There is no need to retry use a
bigger buffer.

 However, as galaxy wrote:
 >    There is no opportunity for the developer to realise a mistake they made
 >    by supplying too small buffer, hence there is no chance of recovering
 >    from it.
 The user gets the wrong data and doesn't realize it.

2. The behavior of getmntent_r in musl libc.
  If the buffer is too small for the line 'overlay', it will set errno
to ERANGE and
  then exit. And the user can retry with a bigger buffer.

  However, as ericonr ever said:
  > The current interface doesn't allow trying again to read the too-long entry,
  > since it will just have moved to the next one.
  The too-long entry data will be lost. Maybe fseek(f, -fgets_result, SEEK_CUR)
  could be used to rewind until the start of the line, but the
function would just
  loop eternally with too short a buffer, for example.
  > The overlay entry has 2000 bytes, the size of buffer is 256 bytes,
if errno is ERANGE
  > i will retry use buffer size 512, 768, 1024, 1280, 1536, 1792,
2048... I have to try at least
  > 8 times to get the cgroup info ! :(

In my opinion, it's ok to truncate the too-long entry (musl libc will
even lose it ! ) instead exiting.
And the errno will be set to ERANGE in order to let uesr realise this
mistake, but as i know
the user can only retry from the beginning of the file, that bothers
me somehow. It‘s up to the user
to decide to retry or not. If the user doesn't care about the too-long
entry or the discarded contents of it,
there is no need to retry. Othrewise, retry from the beginning of the file.

Kaihang