[selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Jan Stancek]

EXT2_* ioctls are likely to fail on other filesystems,
for example: xfs.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 tests/ioctl/Makefile       |    6 ++++++
 tests/ioctl/test_ioctl.c   |    2 ++
 tests/ioctl/test_noioctl.c |    2 ++
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/tests/ioctl/Makefile b/tests/ioctl/Makefile
index 8dce555..def5267 100644
--- a/tests/ioctl/Makefile
+++ b/tests/ioctl/Makefile
@@ -1,4 +1,10 @@
 TARGETS=$(patsubst %.c,%,$(wildcard *.c))
+
+DIR_FS=$(shell df -TP . | awk '{print $2}' | grep 'ext[234]')
+ifneq (x$(DIR_FS),x)
+    CFLAGS += -DUSE_EXTFS_IOCTLS
+endif
+
 all: $(TARGETS)
 clean:
 	rm -f $(TARGETS)
diff --git a/tests/ioctl/test_ioctl.c b/tests/ioctl/test_ioctl.c
index 0852f41..fa5d021 100644
--- a/tests/ioctl/test_ioctl.c
+++ b/tests/ioctl/test_ioctl.c
@@ -49,6 +49,7 @@ int main(int argc, char **argv) {
     exit(1);
   }
 
+#ifdef USE_EXTFS_IOCTLS
   /* This one should hit the FILE__GETATTR or FILE__READ test */
   rc = ioctl(fd, EXT2_IOC_GETVERSION, &val);
   if( rc != 0 ) {
@@ -63,6 +64,7 @@ int main(int argc, char **argv) {
     perror("test_ioctl:EXT2_IOC_SETVERSION");
     exit(1);
   }
+#endif
 
   close(fd);
   exit(0);
diff --git a/tests/ioctl/test_noioctl.c b/tests/ioctl/test_noioctl.c
index ef3fac5..b0ca660 100644
--- a/tests/ioctl/test_noioctl.c
+++ b/tests/ioctl/test_noioctl.c
@@ -68,6 +68,7 @@ int main(int argc, char **argv) {
     exit(1);
   }
 
+#ifdef USE_EXTFS_IOCTLS
   /*
    * This one depends on kernel version:
    * New:  Should hit the FILE__READ test and succeed.
@@ -86,6 +87,7 @@ int main(int argc, char **argv) {
     perror("test_noioctl:EXT2_IOC_SETVERSION");
     exit(1);
   }
+#endif
 
   close(fd);
   exit(0);
-- 
1.7.1


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Stephen Smalley]

On 01/23/2013 08:27 AM, Jan Stancek wrote:
> EXT2_* ioctls are likely to fail on other filesystems,
> for example: xfs.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>   tests/ioctl/Makefile       |    6 ++++++
>   tests/ioctl/test_ioctl.c   |    2 ++
>   tests/ioctl/test_noioctl.c |    2 ++
>   3 files changed, 10 insertions(+), 0 deletions(-)

Shouldn't really be a compile-time flag but rather a runtime test of the 
filesystem type, right?  Otherwise it presumes you built it on the same 
system and in the same filesystem where you will run it.

I thought however that your earlier patch to switch it to using FS_IOC_ 
command values would have
addressed the issue of other filesystem types already?  What error are 
you getting from xfs?  The code already checks for one errno value to 
distinguish the not-supported case.

> diff --git a/tests/ioctl/Makefile b/tests/ioctl/Makefile
> index 8dce555..def5267 100644
> --- a/tests/ioctl/Makefile
> +++ b/tests/ioctl/Makefile
> @@ -1,4 +1,10 @@
>   TARGETS=$(patsubst %.c,%,$(wildcard *.c))
> +
> +DIR_FS=$(shell df -TP . | awk '{print $2}' | grep 'ext[234]')
> +ifneq (x$(DIR_FS),x)
> +    CFLAGS += -DUSE_EXTFS_IOCTLS
> +endif
> +
>   all: $(TARGETS)
>   clean:
>   	rm -f $(TARGETS)
> diff --git a/tests/ioctl/test_ioctl.c b/tests/ioctl/test_ioctl.c
> index 0852f41..fa5d021 100644
> --- a/tests/ioctl/test_ioctl.c
> +++ b/tests/ioctl/test_ioctl.c
> @@ -49,6 +49,7 @@ int main(int argc, char **argv) {
>       exit(1);
>     }
>
> +#ifdef USE_EXTFS_IOCTLS
>     /* This one should hit the FILE__GETATTR or FILE__READ test */
>     rc = ioctl(fd, EXT2_IOC_GETVERSION, &val);
>     if( rc != 0 ) {
> @@ -63,6 +64,7 @@ int main(int argc, char **argv) {
>       perror("test_ioctl:EXT2_IOC_SETVERSION");
>       exit(1);
>     }
> +#endif
>
>     close(fd);
>     exit(0);
> diff --git a/tests/ioctl/test_noioctl.c b/tests/ioctl/test_noioctl.c
> index ef3fac5..b0ca660 100644
> --- a/tests/ioctl/test_noioctl.c
> +++ b/tests/ioctl/test_noioctl.c
> @@ -68,6 +68,7 @@ int main(int argc, char **argv) {
>       exit(1);
>     }
>
> +#ifdef USE_EXTFS_IOCTLS
>     /*
>      * This one depends on kernel version:
>      * New:  Should hit the FILE__READ test and succeed.
> @@ -86,6 +87,7 @@ int main(int argc, char **argv) {
>       perror("test_noioctl:EXT2_IOC_SETVERSION");
>       exit(1);
>     }
> +#endif
>
>     close(fd);
>     exit(0);
>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Jan Stancek]

----- Original Message -----
> From: "Stephen Smalley" <sds@tycho.nsa.gov>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: selinux@tycho.nsa.gov, serge@hallyn.com, jburke@redhat.com
> Sent: Wednesday, 23 January, 2013 4:41:38 PM
> Subject: Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]
> 
> On 01/23/2013 08:27 AM, Jan Stancek wrote:
> > EXT2_* ioctls are likely to fail on other filesystems,
> > for example: xfs.
> >
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >   tests/ioctl/Makefile       |    6 ++++++
> >   tests/ioctl/test_ioctl.c   |    2 ++
> >   tests/ioctl/test_noioctl.c |    2 ++
> >   3 files changed, 10 insertions(+), 0 deletions(-)
> 
> Shouldn't really be a compile-time flag but rather a runtime test of
> the
> filesystem type, right?  Otherwise it presumes you built it on the
> same
> system and in the same filesystem where you will run it.

Correct, I assumed everyone is running it with 'make test'.

> 
> I thought however that your earlier patch to switch it to using
> FS_IOC_
> command values would have
> addressed the issue of other filesystem types already?  What error
> are
> you getting from xfs?

ioctl(3, FS_IOC32_SETVERSION or FS_IOC_SETVERSION or SONYPI_IOCGBAT1CAP, 0x7fffb5eeac24) = -1 ENOTTY (Inappropriate ioctl for device)

> The code already checks for one errno value to
> distinguish the not-supported case.

I can't see any check for errno in this test:
$ grep "errno" -r tests/ioctl/
$

Regards,
Jan

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Stephen Smalley]

[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]

Odd, I have the following diff locally.


On 01/23/2013 11:50 AM, Jan Stancek wrote:
>
>
> ----- Original Message -----
>> From: "Stephen Smalley" <sds@tycho.nsa.gov>
>> To: "Jan Stancek" <jstancek@redhat.com>
>> Cc: selinux@tycho.nsa.gov, serge@hallyn.com, jburke@redhat.com
>> Sent: Wednesday, 23 January, 2013 4:41:38 PM
>> Subject: Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]
>>
>> On 01/23/2013 08:27 AM, Jan Stancek wrote:
>>> EXT2_* ioctls are likely to fail on other filesystems,
>>> for example: xfs.
>>>
>>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>>> ---
>>>    tests/ioctl/Makefile       |    6 ++++++
>>>    tests/ioctl/test_ioctl.c   |    2 ++
>>>    tests/ioctl/test_noioctl.c |    2 ++
>>>    3 files changed, 10 insertions(+), 0 deletions(-)
>>
>> Shouldn't really be a compile-time flag but rather a runtime test of
>> the
>> filesystem type, right?  Otherwise it presumes you built it on the
>> same
>> system and in the same filesystem where you will run it.
>
> Correct, I assumed everyone is running it with 'make test'.
>
>>
>> I thought however that your earlier patch to switch it to using
>> FS_IOC_
>> command values would have
>> addressed the issue of other filesystem types already?  What error
>> are
>> you getting from xfs?
>
> ioctl(3, FS_IOC32_SETVERSION or FS_IOC_SETVERSION or SONYPI_IOCGBAT1CAP, 0x7fffb5eeac24) = -1 ENOTTY (Inappropriate ioctl for device)
>
>> The code already checks for one errno value to
>> distinguish the not-supported case.
>
> I can't see any check for errno in this test:
> $ grep "errno" -r tests/ioctl/
> $
>
> Regards,
> Jan
>
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.
>


[-- Attachment #2: ioctl.diff --]
[-- Type: text/x-patch, Size: 3949 bytes --]

diff --git a/tests/ioctl/test_ioctl.c b/tests/ioctl/test_ioctl.c
index 0852f41..a0fe365 100644
--- a/tests/ioctl/test_ioctl.c
+++ b/tests/ioctl/test_ioctl.c
@@ -6,9 +6,7 @@
 #include <sys/stat.h>
 #include <linux/fs.h>
 #include <unistd.h>
-
-#define EXT2_IOC_GETVERSION FS_IOC_GETVERSION
-#define EXT2_IOC_SETVERSION FS_IOC_SETVERSION
+#include <errno.h>
 
 /*
  * Test the ioctl() calls on a file whose name is given as the first
@@ -30,37 +28,37 @@ int main(int argc, char **argv) {
 
   /* This one should hit the FILE__GETATTR or FILE__IOCTL test */
   rc = ioctl(fd, FIGETBSZ, &val);
-  if( rc != 0 ) {
+  if( rc < 0 ) {
     perror("test_ioctl:FIGETBSZ");
     exit(1);
   }
 
   /* This one should hit the FILE__IOCTL test */
   rc = ioctl(fd, FIOCLEX);
-  if( rc != 0 ) {
+  if( rc < 0 ) {
     perror("test_ioctl:FIOCLEX");
     exit(1);
   }
 
   /* This one should hit the FD__USE or FILE__IOCTL test */
   rc = ioctl(fd, FIONBIO, &val);
-  if( rc != 0 ) {
+  if( rc < 0 ) {
     perror("test_ioctl:FIONBIO");
     exit(1);
   }
 
   /* This one should hit the FILE__GETATTR or FILE__READ test */
-  rc = ioctl(fd, EXT2_IOC_GETVERSION, &val);
-  if( rc != 0 ) {
-    perror("test_ioctl:EXT2_IOC_GETVERSION");
+  rc = ioctl(fd, FS_IOC_GETVERSION, &val);
+  if( rc < 0 && errno != ENOTTY) {
+    perror("test_ioctl:FS_IOC_GETVERSION");
     exit(1);
   }
 
   /* This one should hit the FILE__SETATTR or FILE__WRITE test */
   val = 0;
-  rc = ioctl(fd, EXT2_IOC_SETVERSION, &val);
-  if( rc != 0 ) {
-    perror("test_ioctl:EXT2_IOC_SETVERSION");
+  rc = ioctl(fd, FS_IOC_SETVERSION, &val);
+  if( rc < 0 && errno != ENOTTY) {
+    perror("test_ioctl:FS_IOC_SETVERSION");
     exit(1);
   }
 
diff --git a/tests/ioctl/test_noioctl.c b/tests/ioctl/test_noioctl.c
index ef3fac5..f615054 100644
--- a/tests/ioctl/test_noioctl.c
+++ b/tests/ioctl/test_noioctl.c
@@ -11,9 +11,7 @@
 #include <sys/utsname.h>
 #include <unistd.h>
 #include <string.h>
-
-#define EXT2_IOC_GETVERSION FS_IOC_GETVERSION
-#define EXT2_IOC_SETVERSION FS_IOC_SETVERSION
+#include <errno.h>
 
 /*
  * Test the ioctl() calls on a file whose name is given as the first
@@ -25,7 +23,7 @@
 int main(int argc, char **argv) {
   struct utsname uts;
   int fd;
-  int rc, oldkernel = 1;
+  int rc, useaccessmode = 1;
   int val;
 
   if (uname(&uts) < 0) {
@@ -34,7 +32,7 @@ int main(int argc, char **argv) {
   }
 
   if (strverscmp(uts.release, "2.6.27") >= 0 && strverscmp(uts.release, "2.6.39") < 0)
-    oldkernel = 0;
+    useaccessmode = 0;
 
   fd = open(argv[1], O_RDONLY, 0);
  
@@ -59,11 +57,11 @@ int main(int argc, char **argv) {
 
   /*
    * This one depends on kernel version:
-   * New:  Should hit the FILE__IOCTL test and fail.
-   * Old:  Should only check FD__USE and succeed.
+   * >= 2.6.27 and < 2.6.39:  Should hit the FILE__IOCTL test and fail.
+   * < 2.6.27 or >= 2.6.39:  Should only check FD__USE and succeed.
    */
   rc = ioctl(fd, FIONBIO, &val);
-  if( !rc == !oldkernel ) {
+  if( !rc == !useaccessmode ) {
     printf("test_noioctl:FIONBIO");
     exit(1);
   }
@@ -73,17 +71,18 @@ int main(int argc, char **argv) {
    * New:  Should hit the FILE__READ test and succeed.
    * Old:  Should hit the FILE__GETATTR test and fail.
    */
-  rc = ioctl(fd, EXT2_IOC_GETVERSION, &val);
-  if( !rc != !oldkernel ) {
-    perror("test_noioctl:EXT2_IOC_GETVERSION");
+  rc = ioctl(fd, FS_IOC_GETVERSION, &val);
+  if( (useaccessmode && rc == 0) ||
+      (!useaccessmode && rc < 0 && errno != ENOTTY) ) {
+    perror("test_noioctl:FS_IOC_GETVERSION");
     exit(1);
   }
 
-  /* This one should hit the FILE__WRITE test and fail. */
+  /* This one should hit the FILE__WRITE or FILE_SETATTR test and fail. */
   val = 0;
-  rc = ioctl(fd, EXT2_IOC_SETVERSION, &val);
+  rc = ioctl(fd, FS_IOC_SETVERSION, &val);
   if( rc == 0 ) {
-    perror("test_noioctl:EXT2_IOC_SETVERSION");
+    perror("test_noioctl:FS_IOC_SETVERSION");
     exit(1);
   }
 

Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Jan Stancek]

That diff looks to be addressing same issue, so there shouldn't
be need for another patch.

I'm guessing we are not using same repo.
$ git remote -v
origin  git://git.selinuxproject.org/~serge/selinux-testsuite.git (fetch)
origin  git://git.selinuxproject.org/~serge/selinux-testsuite.git (push)
$ git pull
Already up-to-date.
$ grep "errno" -r tests/ioctl/
$

Can you share your git uri please?

Regards,
Jan

----- Original Message -----
> From: "Stephen Smalley" <sds@tycho.nsa.gov>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: selinux@tycho.nsa.gov, serge@hallyn.com, jburke@redhat.com
> Sent: Wednesday, 23 January, 2013 6:51:04 PM
> Subject: Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]
> 
> Odd, I have the following diff locally.
> 
> 
> On 01/23/2013 11:50 AM, Jan Stancek wrote:
> >
> >
> > ----- Original Message -----
> >> From: "Stephen Smalley" <sds@tycho.nsa.gov>
> >> To: "Jan Stancek" <jstancek@redhat.com>
> >> Cc: selinux@tycho.nsa.gov, serge@hallyn.com, jburke@redhat.com
> >> Sent: Wednesday, 23 January, 2013 4:41:38 PM
> >> Subject: Re: [selinux-testsuite][PATCH] use extfs ioctls only when
> >> running on ext[234]
> >>
> >> On 01/23/2013 08:27 AM, Jan Stancek wrote:
> >>> EXT2_* ioctls are likely to fail on other filesystems,
> >>> for example: xfs.
> >>>
> >>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> >>> ---
> >>>    tests/ioctl/Makefile       |    6 ++++++
> >>>    tests/ioctl/test_ioctl.c   |    2 ++
> >>>    tests/ioctl/test_noioctl.c |    2 ++
> >>>    3 files changed, 10 insertions(+), 0 deletions(-)
> >>
> >> Shouldn't really be a compile-time flag but rather a runtime test
> >> of
> >> the
> >> filesystem type, right?  Otherwise it presumes you built it on the
> >> same
> >> system and in the same filesystem where you will run it.
> >
> > Correct, I assumed everyone is running it with 'make test'.
> >
> >>
> >> I thought however that your earlier patch to switch it to using
> >> FS_IOC_
> >> command values would have
> >> addressed the issue of other filesystem types already?  What error
> >> are
> >> you getting from xfs?
> >
> > ioctl(3, FS_IOC32_SETVERSION or FS_IOC_SETVERSION or
> > SONYPI_IOCGBAT1CAP, 0x7fffb5eeac24) = -1 ENOTTY (Inappropriate
> > ioctl for device)
> >
> >> The code already checks for one errno value to
> >> distinguish the not-supported case.
> >
> > I can't see any check for errno in this test:
> > $ grep "errno" -r tests/ioctl/
> > $
> >
> > Regards,
> > Jan
> >
> > --
> > This message was distributed to subscribers of the selinux mailing
> > list.
> > If you no longer wish to subscribe, send mail to
> > majordomo@tycho.nsa.gov with
> > the words "unsubscribe selinux" without quotes as the message.
> >
> 
> 

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Stephen Smalley]

On 01/23/2013 01:15 PM, Jan Stancek wrote:
> That diff looks to be addressing same issue, so there shouldn't
> be need for another patch.
>
> I'm guessing we are not using same repo.
> $ git remote -v
> origin  git://git.selinuxproject.org/~serge/selinux-testsuite.git (fetch)
> origin  git://git.selinuxproject.org/~serge/selinux-testsuite.git (push)
> $ git pull
> Already up-to-date.
> $ grep "errno" -r tests/ioctl/
> $
>
> Can you share your git uri please?

Same repo, but this is a local change not yet committed in my working 
copy.  I'm guessing that I forgot to send it or it just never got applied.




--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Serge E. Hallyn]

Quoting Stephen Smalley (sds@tycho.nsa.gov):
> Odd, I have the following diff locally.

Hi,

is there any commit message or author attribution for the diff, so I
can apply it to the testsuite?

thanks,
-serge

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [selinux-testsuite][PATCH] use extfs ioctls only when running on ext[234]

[Stephen Smalley]

[-- Attachment #1: Type: text/plain, Size: 254 bytes --]

On 01/23/2013 11:36 PM, Serge E. Hallyn wrote:
> Quoting Stephen Smalley (sds@tycho.nsa.gov):
>> Odd, I have the following diff locally.
>
> Hi,
>
> is there any commit message or author attribution for the diff, so I
> can apply it to the testsuite?




[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Ensure-that-the-ioctl-tests-do-not-fail-on-other-fil.patch --]
[-- Type: text/x-patch; name="0001-Ensure-that-the-ioctl-tests-do-not-fail-on-other-fil.patch", Size: 0 bytes --]