Re: Open master pty (/dev/ptmx) non blocking

2022-09-27 Thread David Holland
On Fri, Sep 23, 2022 at 05:34:26PM -0400, David H. Gutteridge wrote:
 > It's not just O_NONBLOCK that can be expected/desired, vte wants to set
 > O_CLOEXEC as well. The Linux kernel accepts and applies both of those
 > flags in a posix_openpt(3) call.

There should be no path through open(2) where O_CLOEXEC doesn't work!
It is not a property of the underlying object. Or absolutely shouldn't
be, anyway.

-- 
David A. Holland
dholl...@netbsd.org


Re: Open master pty (/dev/ptmx) non blocking

2022-09-24 Thread Christos Zoulas
In article <25389.33419.131713.821...@gargle.gargle.howl>,
Anthony Mallet   wrote:
>-=-=-=-=-=-
>
>Hi,
>
>I have a piece of software that opens a master pty non-blocking:
>fd = open("/dev/ptmx", O_RDWR | O_NOCTTY | O_NONBLOCK);
>
>The intent is to make further read(2) on the master non blocking. But
>the O_NONBLOCK flag seems to be ignored. Attached is a minimal sample
>C program showing the issue.
>
>Several remarks:
>* open(2) manual does not mention a master pty as special regarding the
>  O_NONBLOCK flag, and even says "this flag also has the effect of making
>  all subsequent I/O on the open file non-blocking",
>* explicitly setting the file descriptor as non-blocking with fcntl(2)
>  works fine,
>* a slave pty has no such issue (O_NONBLOCK in open(2) is honoured),
>* FWIW, linux has no surprise here, the flag behaves as one would expect,
>* POSIX does not mention the flag as supported in posix_openpt(3) (it
>  does not says it's not supported either :).
>
>So, I'm not sure if something should be changed here, and if someone
>is willing to check that? I tried to track down where in the kernel
>this happens, maybe in pty_alloc_master in kern/tty_ptm.c? I really
>don't master those kernel aspects so I'm not confident in providing a
>patch.
>
>I guess the principle of least surprise would say that the flag should
>be supported (e.g. for maximum portability). OTOH, if the current
>behaviour is deemed correct or sufficient, maybe at least some manual
>(posix_openpt, open, ...) should mention this is not supported?
>
>Best,
>Anthony

Fixed in HEAD.

christos



Re: Open master pty (/dev/ptmx) non blocking

2022-09-24 Thread Anthony Mallet
On Friday 23 Sep 2022, at 17:34, David H. Gutteridge wrote:
> On Fri, 23 Sep 2022 at 20:14:23 +, David Holland wrote:
> > On Fri, Sep 23, 2022 at 01:39:16PM +0200, Martin Husemann wrote:
> > >  Note that unlike implementations on some other operating
> > >  systems, posix_openpt() does not return EINVAL if the value
> > >  of oflag would be deemed invalid
> >
> > That is, however, kind of a feeble excuse :-)
>
> Agreed. But I inferred this was perhaps done this way on purpose, and
> could be controversial to change.

I noticed that this might also affect Linux emulation as
pty_alloc_master() is used for both native and emualted software.

Linux emulated applications may not be prepared deal with EINVAL if
they pass e.g. O_NONBLOCK.


Re: Open master pty (/dev/ptmx) non blocking

2022-09-24 Thread Anthony Mallet
On Fri, 23 Sep 2022, David Holland wrote:
> While my inclination would be to make it work, until someone wants to
> figure out how to do that it seems straightforward to make O_NONBLOCK
> fail:

Mostly out of curiosity and for the records, I tested the attached
patch. It enables the EINVAL error for unsupported flags and sets
O_NONBLOCK if present. This seems to work after a quick test.

Regarding the O_CLOEXEC, it may require a bit more work. I feel like
the fd_set_exclose() function in kern_descrip.c could be used.


Index: sys/kern/tty_ptm.c
===
RCS file: /cvsroot/src/sys/kern/tty_ptm.c,v
retrieving revision 1.43
diff -u -r1.43 tty_ptm.c
--- sys/kern/tty_ptm.c  29 Jun 2021 22:40:53 -  1.43
+++ sys/kern/tty_ptm.c  24 Sep 2022 15:41:44 -
@@ -87,7 +87,7 @@
 int pts_major, ptc_major;
 
 static dev_t pty_getfree(void);
-static int pty_alloc_master(struct lwp *, int *, dev_t *, struct mount *);
+static int pty_alloc_master(struct lwp *, int, int *, dev_t *, struct mount *);
 static int pty_alloc_slave(struct lwp *, int *, dev_t, struct mount *);
 static int pty_vn_open(struct vnode *, struct lwp *);
 
@@ -155,13 +155,16 @@
 }
 
 static int
-pty_alloc_master(struct lwp *l, int *fd, dev_t *dev, struct mount *mp)
+pty_alloc_master(struct lwp *l, int flag, int *fd, dev_t *dev, struct mount 
*mp)
 {
int error;
struct file *fp;
struct vnode *vp;
int md;
 
+   if (flag & ~(O_ACCMODE | O_NOCTTY | O_NONBLOCK))
+   return EINVAL;
+
if ((error = fd_allocfile(, fd)) != 0) {
DPRINTF(("fd_allocfile %d\n", error));
return error;
@@ -199,7 +202,7 @@
else
goto bad;
}
-   fp->f_flag = FREAD|FWRITE;
+   fp->f_flag = FREAD|FWRITE | (flag & O_NONBLOCK);
fp->f_type = DTYPE_VNODE;
fp->f_ops = 
fp->f_vnode = vp;
@@ -343,7 +346,7 @@
case 2: /* /emul/linux/dev/ptmx */
if ((error = pty_getmp(l, )) != 0)
return error;
-   if ((error = pty_alloc_master(l, , , mp)) != 0)
+   if ((error = pty_alloc_master(l, flag, , , mp)) != 0)
return error;
if (minor(dev) == 2) {
/*
@@ -392,7 +395,7 @@
if ((error = pty_getmp(l, )) != 0)
return error;
 
-   if ((error = pty_alloc_master(l, , , mp)) != 0)
+   if ((error = pty_alloc_master(l, 0, , , mp)) != 0)
return error;
 
if ((error = pty_grant_slave(l, newdev, mp)) != 0)


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread David H. Gutteridge

On Fri, 23 Sep 2022 at 20:14:23 +, David Holland wrote:

On Fri, Sep 23, 2022 at 01:39:16PM +0200, Martin Husemann wrote:
> > Then, shouldn't the open(2) (and posix_openpt(3)) at least fail with
> > EINVAL or something if other flags are specified?
>
> The man page says:
>
>  Note that unlike implementations on some other operating systems,
>  posix_openpt() does not return EINVAL if the value of oflag would be
>  deemed invalid, instead it is simply ignored.  This means it is not
>  possible to dynamically test which open(2) flags are possible to set, and
>  apply a fallback if EINVAL is received.

That is, however, kind of a feeble excuse :-)


Agreed. But I inferred this was perhaps done this way on purpose, and
could be controversial to change. (Would also then mean downstream code
would have to special-case NetBSD releases and/or add tests which could
be cumbersome.) I hadn't got around to asking, I was dealing with
documenting the reality and working with the vte project so that NetBSD
support for gnome-terminal/mate-terminal/xfce4-terminal/et al. was
accepted.


While my inclination would be to make it work, until someone wants to
figure out how to do that it seems straightforward to make O_NONBLOCK
fail:

Index: tty_ptm.c
===
RCS file: /cvsroot/src/sys/kern/tty_ptm.c,v
retrieving revision 1.43
diff -u -p -r1.43 tty_ptm.c
--- tty_ptm.c   29 Jun 2021 22:40:53 -  1.43
+++ tty_ptm.c   23 Sep 2022 20:12:07 -
@@ -338,6 +338,10 @@ ptmopen(dev_t dev, int flag, int mode, s
dev_t ttydev;
struct mount *mp;

+   if (flag & O_NONBLOCK) {
+   return EINVAL;
+   }
+
switch(minor(dev)) {
case 0: /* /dev/ptmx */
case 2: /* /emul/linux/dev/ptmx */


It's not just O_NONBLOCK that can be expected/desired, vte wants to set
O_CLOEXEC as well. The Linux kernel accepts and applies both of those
flags in a posix_openpt(3) call.

OpenBSD does this validation in posix_openpt(3) directly:

/* User must specify O_RDWR in oflag. */
if ((oflag & O_ACCMODE) != O_RDWR ||
(oflag & ~(O_ACCMODE | O_NOCTTY)) != 0) {
errno = EINVAL;
return -1;
}

Now, that doesn't cover the case that someone simply calls open(2) on
the ptmx cloning device directly, so safer to be placed where you're
suggesting. (And I make no claims to being an authority on any of
this, I've simply looked at it a bit, and may well have missed
something.)

Regards,

Dave


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread RVP

On Fri, 23 Sep 2022, David Holland wrote:


While my inclination would be to make it work, until someone wants to
figure out how to do that it seems straightforward to make O_NONBLOCK
fail:

Index: tty_ptm.c
===
RCS file: /cvsroot/src/sys/kern/tty_ptm.c,v
retrieving revision 1.43
diff -u -p -r1.43 tty_ptm.c
--- tty_ptm.c   29 Jun 2021 22:40:53 -  1.43
+++ tty_ptm.c   23 Sep 2022 20:12:07 -
@@ -338,6 +338,10 @@ ptmopen(dev_t dev, int flag, int mode, s
dev_t ttydev;
struct mount *mp;

+   if (flag & O_NONBLOCK) {
+   return EINVAL;
+   }
+



Or:
```
if (flags & ~(O_RDWR | O_NOCTTY))
return EINVAL;
```

-RVP


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread David Holland
On Fri, Sep 23, 2022 at 01:39:16PM +0200, Martin Husemann wrote:
 > > Then, shouldn't the open(2) (and posix_openpt(3)) at least fail with
 > > EINVAL or something if other flags are specified?
 > 
 > The man page says:
 > 
 >  Note that unlike implementations on some other operating systems,
 >  posix_openpt() does not return EINVAL if the value of oflag would be
 >  deemed invalid, instead it is simply ignored.  This means it is not
 >  possible to dynamically test which open(2) flags are possible to set, 
 > and
 >  apply a fallback if EINVAL is received.

That is, however, kind of a feeble excuse :-)

While my inclination would be to make it work, until someone wants to
figure out how to do that it seems straightforward to make O_NONBLOCK
fail:

Index: tty_ptm.c
===
RCS file: /cvsroot/src/sys/kern/tty_ptm.c,v
retrieving revision 1.43
diff -u -p -r1.43 tty_ptm.c
--- tty_ptm.c   29 Jun 2021 22:40:53 -  1.43
+++ tty_ptm.c   23 Sep 2022 20:12:07 -
@@ -338,6 +338,10 @@ ptmopen(dev_t dev, int flag, int mode, s
dev_t ttydev;
struct mount *mp;
 
+   if (flag & O_NONBLOCK) {
+   return EINVAL;
+   }
+
switch(minor(dev)) {
case 0: /* /dev/ptmx */
case 2: /* /emul/linux/dev/ptmx */


-- 
David A. Holland
dholl...@netbsd.org


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread David H. Gutteridge

On Fri, 23 Sep 2022 at 11:49:32 + (UTC), RVP wrote:

On Fri, 23 Sep 2022, Anthony Mallet wrote:

Then, shouldn't the open(2) (and posix_openpt(3)) at least fail with
EINVAL or something if other flags are specified?


Yes, this was noticed recently by gutteridge@ (I think) who also
amended the manpage to what Martin just quoted (and which may not
have been pulled up to -9.x)


The other BSDs validate the oflag argument and return EINVAL if
anything supplied is inapplicable. This is also the case with recent
Solaris derivates (my test reference being OmniOS). What they
accept and where they validate varies, with OpenBSD doing so in
the posix_openpt function, and FreeBSD doing so in a lower layer (not
that that really matters here). This came up recently as I was
reviewing issues with vte support of NetBSD. The upstream was expecting
to be able to test for flag support via EINVAL responses, but that
didn't work for NetBSD, which instead would simply return an FD that
was stuck in an unusable state for their purposes and expectations.

It was on my (long) list of things to do to either mention this on a
mailing list like this or file a PR. It's not wrong from a POSIX
perspective as I read it. I'm not sure if there would be an appetite to
change it now -- consider this my question to the list. :)

I hadn't submitted a pullup request to update the man pages on stable
releases, as I wanted to ask about it first, and it seemed kind of
obscure (this is an unexpected coincidence). I can certainly do so.

Regards,

Dave


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread RVP

On Fri, 23 Sep 2022, Anthony Mallet wrote:


Then, shouldn't the open(2) (and posix_openpt(3)) at least fail with
EINVAL or something if other flags are specified?



Yes, this was noticed recently by gutteridge@ (I think) who also
amended the manpage to what Martin just quoted (and which may not
have been pulled up to -9.x)

-RVP



Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread Anthony Mallet
> The man page says:
> 
>  Note that unlike implementations on some other operating systems,
>  posix_openpt() does not return EINVAL if the value of oflag would
>  be deemed invalid

Oh, right. I missed that...

I see that this was added 2 weeks ago and my base is from last March
only. While I did check that the code did not change, I did not look
for recent commits in the manual.

Allrighty, sorry for the noise!

Cheers,
Anthony


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread Martin Husemann
On Fri, Sep 23, 2022 at 01:26:00PM +0200, Anthony Mallet wrote:
> On Friday 23 Sep 2022, at 10:29, RVP wrote:
> > So, O_NONBLOCK is, at least, _definitely_ non-portable. Best to use
> > fcntl() here and not depend on a Linux-specific behaviour.
> 
> Fair enough :)
> 
> Then, shouldn't the open(2) (and posix_openpt(3)) at least fail with
> EINVAL or something if other flags are specified?

The man page says:

 Note that unlike implementations on some other operating systems,
 posix_openpt() does not return EINVAL if the value of oflag would be
 deemed invalid, instead it is simply ignored.  This means it is not
 possible to dynamically test which open(2) flags are possible to set, and
 apply a fallback if EINVAL is received.

Martin


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread Anthony Mallet
On Friday 23 Sep 2022, at 10:29, RVP wrote:
> So, O_NONBLOCK is, at least, _definitely_ non-portable. Best to use
> fcntl() here and not depend on a Linux-specific behaviour.

Fair enough :)

Then, shouldn't the open(2) (and posix_openpt(3)) at least fail with
EINVAL or something if other flags are specified?


Re: Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread RVP

On Fri, 23 Sep 2022, Anthony Mallet wrote:


* POSIX does not mention the flag as supported in posix_openpt(3) (it
 does not says it's not supported either :).



According to POSIX, anything other than O_RDWR or O_NOCTTY is "unspecified";
and, the FreeBSD man page explicitly states:

 O_RDWR Open for reading and writing.

 O_NOCTTY   If set posix_openpt() shall not cause the terminal
device to become the controlling terminal for the
process.

 O_CLOEXEC  Set the close-on-exec flag for the new file
descriptor.

 The posix_openpt() function shall fail when oflag contains other
 values.

So, O_NONBLOCK is, at least, _definitely_ non-portable. Best to use fcntl()
here and not depend on a Linux-specific behaviour.

-RVP


Open master pty (/dev/ptmx) non blocking

2022-09-23 Thread Anthony Mallet
Hi,

I have a piece of software that opens a master pty non-blocking:
fd = open("/dev/ptmx", O_RDWR | O_NOCTTY | O_NONBLOCK);

The intent is to make further read(2) on the master non blocking. But
the O_NONBLOCK flag seems to be ignored. Attached is a minimal sample
C program showing the issue.

Several remarks:
* open(2) manual does not mention a master pty as special regarding the
  O_NONBLOCK flag, and even says "this flag also has the effect of making
  all subsequent I/O on the open file non-blocking",
* explicitly setting the file descriptor as non-blocking with fcntl(2)
  works fine,
* a slave pty has no such issue (O_NONBLOCK in open(2) is honoured),
* FWIW, linux has no surprise here, the flag behaves as one would expect,
* POSIX does not mention the flag as supported in posix_openpt(3) (it
  does not says it's not supported either :).

So, I'm not sure if something should be changed here, and if someone
is willing to check that? I tried to track down where in the kernel
this happens, maybe in pty_alloc_master in kern/tty_ptm.c? I really
don't master those kernel aspects so I'm not confident in providing a
patch.

I guess the principle of least surprise would say that the flag should
be supported (e.g. for maximum portability). OTOH, if the current
behaviour is deemed correct or sufficient, maybe at least some manual
(posix_openpt, open, ...) should mention this is not supported?

Best,
Anthony

#include 

#include 
#include 
#include 

int
main()
{
  char buf[8];
  ssize_t s;
  int fd;

  /* open non-blocking: ignored */
  fd = open("/dev/ptmx", O_RDWR | O_NOCTTY | O_NONBLOCK);
  if (fd < 0) err(2, "/dev/ptmx");

#if 0
  /* set non-blocking: OK */
  int flag;
  flag = fcntl(fd, F_GETFL);
  if (flag == -1) err(2, "F_GETFL");

  fcntl(fd, F_SETFL, flag | O_NONBLOCK);
#endif

  /* read */
  s = read(fd, buf, sizeof(buf));
  warn("read %zd", s);

  return 0;
}