[LTP] [PATCH 0/3] ioctl01: device check, enable

[LTP] [PATCH 0/3] ioctl01: device check, enable

[Petr Vorel]

Hi,

NOTE: the same should be done for ioctl02, but it should be first
converted to the new API.

Kind regards,
Petr

Petr Vorel (3):
  ioctl01: Add default tty device
  ioctl01: Check tty device
  runtest/syscalls: Enable ioctl01

 runtest/syscalls                          |  2 +-
 testcases/kernel/syscalls/ioctl/ioctl01.c | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] ioctl01: Add default tty device

[Petr Vorel]

Saves user to specify it when run manually.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/ioctl/ioctl01.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c b/testcases/kernel/syscalls/ioctl/ioctl01.c
index 1be38e79d3..cb184aee40 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl01.c
@@ -27,6 +27,7 @@
 #include "lapi/ioctl.h"
 
 #define	INVAL_IOCTL	9999999
+#define	DEFAULT_TTY_DEVICE	"/dev/tty0"
 
 static int fd, fd_file;
 static int bfd = -1;
@@ -70,7 +71,9 @@ static void verify_ioctl(unsigned int i)
 static void setup(void)
 {
 	if (!device)
-		tst_brk(TBROK, "You must specify a tty device with -D option");
+		device = DEFAULT_TTY_DEVICE;
+
+	tst_res(TINFO, "Using device '%s'", device);
 
 	fd = SAFE_OPEN(device, O_RDWR, 0777);
 	fd_file = SAFE_OPEN("x", O_CREAT, 0777);
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] ioctl01: Check tty device

[Petr Vorel]

tty device must be a char device.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/ioctl/ioctl01.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c b/testcases/kernel/syscalls/ioctl/ioctl01.c
index cb184aee40..5e121ae460 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl01.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl01.c
@@ -25,6 +25,7 @@
 #include <termios.h>
 #include "tst_test.h"
 #include "lapi/ioctl.h"
+#include "tst_safe_macros.h"
 
 #define	INVAL_IOCTL	9999999
 #define	DEFAULT_TTY_DEVICE	"/dev/tty0"
@@ -70,11 +71,20 @@ static void verify_ioctl(unsigned int i)
 
 static void setup(void)
 {
+	struct stat st;
+
 	if (!device)
 		device = DEFAULT_TTY_DEVICE;
 
 	tst_res(TINFO, "Using device '%s'", device);
 
+	if (access(device, F_OK))
+		tst_brk(TCONF, "Device '%s' does not exist", device);
+
+	SAFE_STAT(device, &st);
+	if (!S_ISCHR(st.st_mode))
+		tst_brk(TCONF, "Device '%s' is not a char device", device);
+
 	fd = SAFE_OPEN(device, O_RDWR, 0777);
 	fd_file = SAFE_OPEN("x", O_CREAT, 0777);
 }
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] runtest/syscalls: Enable ioctl01

[Petr Vorel]

ioctl01 needs tty device. We check that in 2 previous commits,
thus it can be safely enabled by default.

NOTE: the same should be done for ioctl02, but it should be first
converted to the new API.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 runtest/syscalls | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/runtest/syscalls b/runtest/syscalls
index ae37a1192a..c4287c3e90 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -543,8 +543,8 @@ getxattr05 getxattr05
 init_module01 init_module01
 init_module02 init_module02
 
+ioctl01 ioctl01
 #Needs tty device.
-#ioctl01 ioctl01 -D /dev/tty0
 #ioctl02 ioctl02 -D /dev/tty0
 
 # Introducing ioctl tests for all /dev/tty* devices
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Li Wang]

Hi Petr,

On Tue, Feb 7, 2023 at 9:17 PM Petr Vorel <pvorel@suse.cz> wrote:

> Saves user to specify it when run manually.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/kernel/syscalls/ioctl/ioctl01.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl01.c
> b/testcases/kernel/syscalls/ioctl/ioctl01.c
> index 1be38e79d3..cb184aee40 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl01.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl01.c
> @@ -27,6 +27,7 @@
>  #include "lapi/ioctl.h"
>
>  #define        INVAL_IOCTL     9999999
> +#define        DEFAULT_TTY_DEVICE      "/dev/tty0"
>

Hidden the device path parameter is a good idea.

But maybe can we add a function to find available char devices instead
of using the tty0 as default? In that function, we do the S_ISCHR() check
and return the valid path of it. Then the rest test (e.g. ioctl02) can make
use of it but not set the specified device as well. WDYT?



>
>  static int fd, fd_file;
>  static int bfd = -1;
> @@ -70,7 +71,9 @@ static void verify_ioctl(unsigned int i)
>  static void setup(void)
>  {
>         if (!device)
> -               tst_brk(TBROK, "You must specify a tty device with -D
> option");
> +               device = DEFAULT_TTY_DEVICE;
> +
> +       tst_res(TINFO, "Using device '%s'", device);
>
>         fd = SAFE_OPEN(device, O_RDWR, 0777);
>         fd_file = SAFE_OPEN("x", O_CREAT, 0777);
> --
> 2.39.1
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Petr Vorel]

Hi Li,

> > +#define        DEFAULT_TTY_DEVICE      "/dev/tty0"

> Hidden the device path parameter is a good idea.

> But maybe can we add a function to find available char devices instead
> of using the tty0 as default? In that function, we do the S_ISCHR() check
> and return the valid path of it. Then the rest test (e.g. ioctl02) can make
> use of it but not set the specified device as well. WDYT?

FYI I'm using S_ISCHR() in other patches, which check if device can be used.
Implementing search looks like a good idea. Are useful files any /dev/tty*
(including /dev/tty, /dev/ttyACM0, /dev/ttyS0) or should I avoid any file
or include other paths?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Petr Vorel]

> Hi Li,

> > > +#define        DEFAULT_TTY_DEVICE      "/dev/tty0"

> > Hidden the device path parameter is a good idea.

> > But maybe can we add a function to find available char devices instead
> > of using the tty0 as default? In that function, we do the S_ISCHR() check
> > and return the valid path of it. Then the rest test (e.g. ioctl02) can make
> > use of it but not set the specified device as well. WDYT?

> FYI I'm using S_ISCHR() in other patches, which check if device can be used.
> Implementing search looks like a good idea. Are useful files any /dev/tty*
> (including /dev/tty, /dev/ttyACM0, /dev/ttyS0) or should I avoid any file
> or include other paths?

I also wonder if we still want to keep -D parameter (i.e. allow tester to pass a
file).

Kind regards,
Petr

> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Li Wang]

On Wed, Feb 8, 2023 at 9:54 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > > +#define        DEFAULT_TTY_DEVICE      "/dev/tty0"
>
> > Hidden the device path parameter is a good idea.
>
> > But maybe can we add a function to find available char devices instead
> > of using the tty0 as default? In that function, we do the S_ISCHR() check
> > and return the valid path of it. Then the rest test (e.g. ioctl02) can
> make
> > use of it but not set the specified device as well. WDYT?
>
> FYI I'm using S_ISCHR() in other patches, which check if device can be
> used.
> Implementing search looks like a good idea. Are useful files any /dev/tty*
> (including /dev/tty, /dev/ttyACM0, /dev/ttyS0) or should I avoid any file
> or include other paths?
>

It seems not all char devices can be used here.
The /dev/tty*Num* should be the first choice.

I tried some of them and only /dev/tty0-N works well,
maybe we can just make use of them to avoid no tty0
file failure should be enough. Because my only concern
about the hard coding is that "/dev/tty0" is non-exist.



>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Li Wang]

On Wed, Feb 8, 2023 at 10:13 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Li,
>
> > > > +#define        DEFAULT_TTY_DEVICE      "/dev/tty0"
>
> > > Hidden the device path parameter is a good idea.
>
> > > But maybe can we add a function to find available char devices instead
> > > of using the tty0 as default? In that function, we do the S_ISCHR()
> check
> > > and return the valid path of it. Then the rest test (e.g. ioctl02) can
> make
> > > use of it but not set the specified device as well. WDYT?
>
> > FYI I'm using S_ISCHR() in other patches, which check if device can be
> used.
> > Implementing search looks like a good idea. Are useful files any
> /dev/tty*
> > (including /dev/tty, /dev/ttyACM0, /dev/ttyS0) or should I avoid any file
> > or include other paths?
>
> I also wonder if we still want to keep -D parameter (i.e. allow tester to
> pass a
> file).
>

I think there is no necessary keep "-D" parameter since this ioctl01
is to check errno of ioctl(2) syscall, it might meaningless to specify a
different char device.


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Petr Vorel]

...
> > FYI I'm using S_ISCHR() in other patches, which check if device can be
> > used.
> > Implementing search looks like a good idea. Are useful files any /dev/tty*
> > (including /dev/tty, /dev/ttyACM0, /dev/ttyS0) or should I avoid any file
> > or include other paths?


> It seems not all char devices can be used here.
> The /dev/tty*Num* should be the first choice.
+1

I'd also use /dev/tty (in case there is no tty0).

> I tried some of them and only /dev/tty0-N works well,
> maybe we can just make use of them to avoid no tty0
> file failure should be enough. Because my only concern
> about the hard coding is that "/dev/tty0" is non-exist.

FYI On my system ioctl01 fail on /dev/ttyS0-N (EIO instead of other expected
errors). The rest /dev/tty, /dev/tty0-N, /dev/ttyACM0-N work.
/dev/ttyACM0-N are just on one of my system, let's not use it.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Petr Vorel]

...
> > I also wonder if we still want to keep -D parameter (i.e. allow tester to
> > pass a
> > file).


> I think there is no necessary keep "-D" parameter since this ioctl01
> is to check errno of ioctl(2) syscall, it might meaningless to specify a
> different char device.

Thx! That makes checking code slightly simpler.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Richard Palethorpe]

Petr Vorel <pvorel@suse.cz> writes:

> Hi Li,
>
>> > +#define        DEFAULT_TTY_DEVICE      "/dev/tty0"
>
>> Hidden the device path parameter is a good idea.
>
>> But maybe can we add a function to find available char devices
>instead

There is already something like this built into the kernel; you can
create a PTY on demand with /dev/ptmx.

See the kernel/pty tests.

>> of using the tty0 as default? In that function, we do the S_ISCHR() check
>> and return the valid path of it. Then the rest test (e.g. ioctl02) can make
>> use of it but not set the specified device as well. WDYT?
>
> FYI I'm using S_ISCHR() in other patches, which check if device can be used.
> Implementing search looks like a good idea. Are useful files any /dev/tty*
> (including /dev/tty, /dev/ttyACM0, /dev/ttyS0) or should I avoid any file
> or include other paths?

These are real serial devices except /dev/tty which could be a real
device or a pty IIUC. Same goes for /dev/hvc[0-9] and possibly some
others.

I'm going to put the patch set to changes requested because /dev/tty0 is
the current virtual console. It seems the test just overwrites the
permissions and starts sending ioctls to it.

I don't know if this is safe and probably it's no different from
creating a pty.

>
> Kind regards,
> Petr


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] ioctl01: Add default tty device

[Petr Vorel]

> Petr Vorel <pvorel@suse.cz> writes:

> > Hi Li,

> >> > +#define        DEFAULT_TTY_DEVICE      "/dev/tty0"

> >> Hidden the device path parameter is a good idea.

> >> But maybe can we add a function to find available char devices
> >instead

> There is already something like this built into the kernel; you can
> create a PTY on demand with /dev/ptmx.

> See the kernel/pty tests.

Thanks for a tip! According to man ptmx(4) the file /dev/ptmx creates
pseudoterminal (PTYs) in /dev/pts directory (e.g. /dev/pts/0).

> >> of using the tty0 as default? In that function, we do the S_ISCHR() check
> >> and return the valid path of it. Then the rest test (e.g. ioctl02) can make
> >> use of it but not set the specified device as well. WDYT?

> > FYI I'm using S_ISCHR() in other patches, which check if device can be used.
> > Implementing search looks like a good idea. Are useful files any /dev/tty*
> > (including /dev/tty, /dev/ttyACM0, /dev/ttyS0) or should I avoid any file
> > or include other paths?
BTW /dev/ttySN are UART serial port TTY (it was familiar to me, although I
nowadays use /dev/ttyUSB0 :)).

> These are real serial devices except /dev/tty which could be a real
> device or a pty IIUC. Same goes for /dev/hvc[0-9] and possibly some
> others.

> I'm going to put the patch set to changes requested because /dev/tty0 is
> the current virtual console. It seems the test just overwrites the
> permissions and starts sending ioctls to it.

Sure.

> I don't know if this is safe and probably it's no different from
> creating a pty.

Kernel doc [1] mentions more about the devices. I suppose it does not
matter for ioctl01 purpose, which TTY device we use, so I'll use
your suggestion.

Kind regards,
Petr

[1] https://www.kernel.org/doc/html/latest/admin-guide/devices.html

> > Kind regards,
> > Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp