Bug#911334: [Piuparts-devel] Bug#911334: Create /dev/ptmx like debootstrap does
On Mon, Oct 29, 2018 at 06:33:50PM +0100, Mathieu Parent wrote: > This was hard work for me (I even wrote a minimum C program), but I've > learnt more things. :) nice. > > If not, I suppose the changelog entry should be clarified... > Don't know. I'll guess we'll leave things as they are now. Thanks again for both your and Andreas' feedback. -- cheers, Holger --- holger@(debian|reproducible-builds|layer-acht).org PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C signature.asc Description: PGP signature
Bug#911334: [Piuparts-devel] Bug#911334: Create /dev/ptmx like debootstrap does
Le lun. 29 oct. 2018 à 17:25, Holger Levsen a écrit : > > Hi Mathieu, Hi Holger, > On Sun, Oct 28, 2018 at 09:43:50PM +0100, Mathieu Parent wrote: > > Thanks. I've done the findutils bug (#912180, with even a patch) and > > I've updated the MR (see also the attached patch). > > > > If the commit message is too verbose, you can amend it. > > that's all cool, now, thanks a lot for your patience & for following up > nicely! This was hard work for me (I even wrote a minimum C program), but I've learnt more things. > I've merged your work now and will deploy it to piuparts.d.o after > sending this email. Just one question: Thanks > > + [ Mathieu Parent ] > > + * piuparts.py: Use mknod instead of touch to create missing /dev/ptmx > > +mountpoint, working around findutils bug #912180 (Closes: #911334) > > does that mean, that once #912180 has been fixed, we can and should > revert a99ecade (=your commit) ? I don't know. Maybe the proper behavior of piuparts should match debootstrap's. This willwork with any dist given kernel is >= 3.16. The kernel doc, says either mknod, or symlink or bindmount. piuparts now (after this merge) does mknod+bindmount. https://www.kernel.org/doc/Documentation/filesystems/devpts.txt > If not, I suppose the changelog entry should be clarified... Don't know. -- Mathieu
Bug#911334: [Piuparts-devel] Bug#911334: Create /dev/ptmx like debootstrap does
On 2018-10-29 18:05, Holger Levsen wrote: > On Mon, Oct 29, 2018 at 05:48:21PM +0100, Andreas Beckmann wrote: >> Since the bug is probably present in all old findutils versions >> (including archived releases) and therefore not getting fixed >> *everywhere*, > > I'm a firm believer in not carry compatibility code for ever. piuparts > should be able to test archived releases, but it doesnt have to run on > it. The script using "find" and triggering the error is run inside the chroot ... But in this case we have two choices that are theoretically equivalent and I would consider neither to be a workaround. It just happens that one of them exposes a bug and the other one therefore works "better". Andreas
Bug#911334: [Piuparts-devel] Bug#911334: Create /dev/ptmx like debootstrap does
Hi Mathieu, On Sun, Oct 28, 2018 at 09:43:50PM +0100, Mathieu Parent wrote: > Thanks. I've done the findutils bug (#912180, with even a patch) and > I've updated the MR (see also the attached patch). > > If the commit message is too verbose, you can amend it. that's all cool, now, thanks a lot for your patience & for following up nicely! I've merged your work now and will deploy it to piuparts.d.o after sending this email. Just one question: > + [ Mathieu Parent ] > + * piuparts.py: Use mknod instead of touch to create missing /dev/ptmx > +mountpoint, working around findutils bug #912180 (Closes: #911334) does that mean, that once #912180 has been fixed, we can and should revert a99ecade (=your commit) ? If not, I suppose the changelog entry should be clarified... -- cheers, Holger --- holger@(debian|reproducible-builds|layer-acht).org PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C signature.asc Description: PGP signature
Bug#911334: [Piuparts-devel] Bug#911334: Create /dev/ptmx like debootstrap does
Le ven. 19 oct. 2018 à 10:05, Andreas Beckmann a écrit :
[...]
> >
> > In this case, piuparts does something like this:
> >
> > touch /dev/ptmx
> > mount -o bind /dev/pts/ptmx /dev/ptmx
>
> That sounds wrong, since we have
>
> self.mount("devpts", "/dev/pts", fstype="devpts",
> opts="newinstance,noexec,nosuid,gid=5,mode=0620,ptmxmode=0666")
> self.mount(self.relative("dev/pts/ptmx"), "/dev/ptmx", opts="bind",
> no_mkdir=True)
>
> and your change would take the /dev/pts/ptmx from the host.
Not from the host, as chroot/dev/ptmx mount is not changed and has "newinstance"
""
To retain backwards compatibility the a ptmx device node (aka any node
created with "mknod name c 5 2") when opened will look for an instance
of devpts under the name "pts" in the same directory as the ptmx device
node.
""
(See https://www.kernel.org/doc/Documentation/filesystems/devpts.txt)
> > The kernel doc [devpts.txt] recommends instead:
> >
> > mknod /dev/ptmx c 5 2
> >
> > And this is what debootstrap does [debootstrap].
>
> as can be seen in the piuparts chroot tarball:
>
> $ less sid_amd64.tar.gz | grep /dev/
> drwxr-xr-x root/root 0 2018-10-18 20:36 ./dev/
> crw--w piupartss/tty 136,8 2018-10-18 20:29 ./dev/console
> lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/stderr ->
> /proc/self/fd/2
> lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/stdout ->
> /proc/self/fd/1
> lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/stdin -> /proc/self/fd/0
> lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/fd -> /proc/self/fd
> crw-rw-rw- root/root 5,2 2018-10-18 20:36 ./dev/ptmx
> drwxrwxrwt root/root 0 2018-10-18 20:36 ./dev/shm/
> drwxr-xr-x root/root 0 2018-10-18 20:36 ./dev/pts/
> crw-rw-rw- root/root 5,0 2018-10-18 20:31 ./dev/tty
> crw-rw-rw- root/root 1,9 2018-10-18 20:31 ./dev/urandom
> crw-rw-rw- root/root 1,8 2018-10-18 20:31 ./dev/random
> crw-rw-rw- root/root 1,7 2018-10-18 20:31 ./dev/full
> crw-rw-rw- root/root 1,5 2018-10-18 20:31 ./dev/zero
> crw-rw-rw- root/root 1,3 2018-10-18 20:31 ./dev/null
>
> and piuparts mounts the ptmx from $chroot/dev/pts/ptmx
> (newinstance devpts) over $chroot/dev/ptmx
If you want to keep this, I can do this.
> > I propose to map the piuparts behavior with debootstrap's.
> >
> > My proposed change is at
> > https://salsa.debian.org/debian/piuparts/merge_requests/2
>
> NACK until I understand the problem.
>
> Might we actually have two different problems here?
>
> * dev/ptmx not being set up correctly in the chroot
It is not setup at all. Neither is dev/pts.
> * something throwing an insecure permissions error?
I don't think so.
Will propose another change.
--
Mathieu Parent
Bug#911334: [Piuparts-devel] Bug#911334: Create /dev/ptmx like debootstrap does
On 2018-10-18 21:01, Mathieu Parent wrote:
> Package: piuparts
> Version: 0.92
> Tag: patch
>
> Hello,
>
> When using piuparts on a chroot without /dev/ptmx [noptmx],
> scripts/pre_remove_50_find_bad_permissions fails with:
>
> ERROR: BAD PERMISSIONS
> crw-rw-rw-. 1 root root 5, 2 Oct 16 03:49 /dev/ptmx
I have this in the host/chroot:
$ l -i /srv/piuparts/tmp/tmpLWxqqg/dev/ptmx
/srv/piuparts/tmp/tmpLWxqqg/dev/pts/ptmx /dev/ptmx /dev/pts/ptmx
1106 crw-rw-rw- 1 root tty 5, 2 Oct 19 09:51 /dev/ptmx
2 c- 1 root root 5, 2 Apr 25 14:45 /dev/pts/ptmx
2 crw-rw-rw- 1 root root 5, 2 Oct 19 09:16
/srv/piuparts/tmp/tmpLWxqqg/dev/ptmx
2 crw-rw-rw- 1 root root 5, 2 Oct 19 09:16
/srv/piuparts/tmp/tmpLWxqqg/dev/pts/ptmx
$ mount | grep tmpLWxqqg
proc on /srv/piuparts/tmp/tmpLWxqqg/proc type proc (rw,relatime)
devpts on /srv/piuparts/tmp/tmpLWxqqg/dev/pts type devpts
(rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=666)
devpts on /srv/piuparts/tmp/tmpLWxqqg/dev/ptmx type devpts
(rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=666)
devpts on /srv/piuparts/tmp/tmpLWxqqg/dev/console type devpts
(rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000)
tmpfs on /srv/piuparts/tmp/tmpLWxqqg/dev/shm type tmpfs
(rw,relatime,size=65536k)
and do not get the error.
>
> In this case, piuparts does something like this:
>
> touch /dev/ptmx
> mount -o bind /dev/pts/ptmx /dev/ptmx
That sounds wrong, since we have
self.mount("devpts", "/dev/pts", fstype="devpts",
opts="newinstance,noexec,nosuid,gid=5,mode=0620,ptmxmode=0666")
self.mount(self.relative("dev/pts/ptmx"), "/dev/ptmx", opts="bind",
no_mkdir=True)
and your change would take the /dev/pts/ptmx from the host.
> The kernel doc [devpts.txt] recommends instead:
>
> mknod /dev/ptmx c 5 2
>
> And this is what debootstrap does [debootstrap].
as can be seen in the piuparts chroot tarball:
$ less sid_amd64.tar.gz | grep /dev/
drwxr-xr-x root/root 0 2018-10-18 20:36 ./dev/
crw--w piupartss/tty 136,8 2018-10-18 20:29 ./dev/console
lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/stderr -> /proc/self/fd/2
lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/stdout -> /proc/self/fd/1
lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/stdin -> /proc/self/fd/0
lrwxrwxrwx root/root 0 2018-10-18 20:31 ./dev/fd -> /proc/self/fd
crw-rw-rw- root/root 5,2 2018-10-18 20:36 ./dev/ptmx
drwxrwxrwt root/root 0 2018-10-18 20:36 ./dev/shm/
drwxr-xr-x root/root 0 2018-10-18 20:36 ./dev/pts/
crw-rw-rw- root/root 5,0 2018-10-18 20:31 ./dev/tty
crw-rw-rw- root/root 1,9 2018-10-18 20:31 ./dev/urandom
crw-rw-rw- root/root 1,8 2018-10-18 20:31 ./dev/random
crw-rw-rw- root/root 1,7 2018-10-18 20:31 ./dev/full
crw-rw-rw- root/root 1,5 2018-10-18 20:31 ./dev/zero
crw-rw-rw- root/root 1,3 2018-10-18 20:31 ./dev/null
and piuparts mounts the ptmx from $chroot/dev/pts/ptmx
(newinstance devpts) over $chroot/dev/ptmx
> I propose to map the piuparts behavior with debootstrap's.
>
> My proposed change is at
> https://salsa.debian.org/debian/piuparts/merge_requests/2
NACK until I understand the problem.
Might we actually have two different problems here?
* dev/ptmx not being set up correctly in the chroot
* something throwing an insecure permissions error?
Andreas
PS: IIRC the pts/ptx/console bits are inspired by the handling in pbuilder
Bug#911334: [Piuparts-devel] Bug#911334: Create /dev/ptmx like debootstrap does
What was the story about the find command failing? Andreas
