[PATCH RFC] selinux: provide __le variables explicitly

[PATCH RFC] selinux: provide __le variables explicitly

[Nicholas Mc Guire]

While the endiannes is being handled properly sparse was unable to verify
this due to type inconsistency. So introduce an additional __le32
respectively _le64 variable to be passed to le32/64_to_cpu() to allow
sparse to verify proper typing. Note that this patch does not change
the generated binary on little-endian systems - on 32bit powerpc it
does change the binary.

Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---

Problem located by an experimental coccinelle script to locate
patters that make sparse unhappy (false positives):

sparse complaints on different architectures fixed by this patch are:

ppc6xx_defconfig
  CHECK   security/selinux/ss/ebitmap.c
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64

Little-endian systems:

loongson3_defconfig
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64

x86_64_defconfig
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64

Patch was compile-tested with: x86_64_defconfig,loongson3_defconfig (both
little-endian) and ppc603_defconfig (big-endian).

On little-endian systems the patch has no impact on the generated binary 
(which is expected) but on the 32bit powerpc it does change the binary
which is not expected but since I'm not able to generate the .lst files
in security/selinux/ss/ due to the lack of a Makefile it is not clear
if this is an unexpected side-effect or due only to the introduction of
the additional variables. From my understanding the patch does not change
the program logic so if the code was correct on big-endian systems before
it should still be correct now.

Patch is against 5.1 (localversion-next is next-20190506)

 security/selinux/ss/ebitmap.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index 8f624f8..09929fc 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -347,7 +347,9 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 {
 	struct ebitmap_node *n = NULL;
 	u32 mapunit, count, startbit, index;
+	__le32 ebitmap_start;
 	u64 map;
+	__le64 mapbits;
 	__le32 buf[3];
 	int rc, i;
 
@@ -381,12 +383,12 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 		goto bad;
 
 	for (i = 0; i < count; i++) {
-		rc = next_entry(&startbit, fp, sizeof(u32));
+		rc = next_entry(&ebitmap_start, fp, sizeof(u32));
 		if (rc < 0) {
 			pr_err("SELinux: ebitmap: truncated map\n");
 			goto bad;
 		}
-		startbit = le32_to_cpu(startbit);
+		startbit = le32_to_cpu(ebitmap_start);
 
 		if (startbit & (mapunit - 1)) {
 			pr_err("SELinux: ebitmap start bit (%d) is "
@@ -423,12 +425,12 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 			goto bad;
 		}
 
-		rc = next_entry(&map, fp, sizeof(u64));
+		rc = next_entry(&mapbits, fp, sizeof(u64));
 		if (rc < 0) {
 			pr_err("SELinux: ebitmap: truncated map\n");
 			goto bad;
 		}
-		map = le64_to_cpu(map);
+		map = le64_to_cpu(mapbits);
 
 		index = (startbit - n->startbit) / EBITMAP_UNIT_SIZE;
 		while (map) {
-- 
2.1.4

Re: [PATCH RFC] selinux: provide __le variables explicitly

[Paul Moore]

On Wed, May 8, 2019 at 2:27 AM Nicholas Mc Guire <hofrat@osadl.org> wrote:
> While the endiannes is being handled properly sparse was unable to verify
> this due to type inconsistency. So introduce an additional __le32
> respectively _le64 variable to be passed to le32/64_to_cpu() to allow
> sparse to verify proper typing. Note that this patch does not change
> the generated binary on little-endian systems - on 32bit powerpc it
> does change the binary.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
>
> Problem located by an experimental coccinelle script to locate
> patters that make sparse unhappy (false positives):
>
> sparse complaints on different architectures fixed by this patch are:
>
> ppc6xx_defconfig
>   CHECK   security/selinux/ss/ebitmap.c
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
>
> Little-endian systems:
>
> loongson3_defconfig
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
>
> x86_64_defconfig
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
>
> Patch was compile-tested with: x86_64_defconfig,loongson3_defconfig (both
> little-endian) and ppc603_defconfig (big-endian).
>
> On little-endian systems the patch has no impact on the generated binary
> (which is expected) but on the 32bit powerpc it does change the binary
> which is not expected but since I'm not able to generate the .lst files
> in security/selinux/ss/ due to the lack of a Makefile it is not clear
> if this is an unexpected side-effect or due only to the introduction of
> the additional variables. From my understanding the patch does not change
> the program logic so if the code was correct on big-endian systems before
> it should still be correct now.

This is a bit worrisome, but I tend to agree that this patch *should*
be correct.  I'm thinking you're probably right in that the resulting
binary difference could be due to the extra variable.  Have you tried
any other big-endian arches?

> Patch is against 5.1 (localversion-next is next-20190506)
>
>  security/selinux/ss/ebitmap.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
> index 8f624f8..09929fc 100644
> --- a/security/selinux/ss/ebitmap.c
> +++ b/security/selinux/ss/ebitmap.c
> @@ -347,7 +347,9 @@ int ebitmap_read(struct ebitmap *e, void *fp)
>  {
>         struct ebitmap_node *n = NULL;
>         u32 mapunit, count, startbit, index;
> +       __le32 ebitmap_start;
>         u64 map;
> +       __le64 mapbits;
>         __le32 buf[3];
>         int rc, i;
>
> @@ -381,12 +383,12 @@ int ebitmap_read(struct ebitmap *e, void *fp)
>                 goto bad;
>
>         for (i = 0; i < count; i++) {
> -               rc = next_entry(&startbit, fp, sizeof(u32));
> +               rc = next_entry(&ebitmap_start, fp, sizeof(u32));
>                 if (rc < 0) {
>                         pr_err("SELinux: ebitmap: truncated map\n");
>                         goto bad;
>                 }
> -               startbit = le32_to_cpu(startbit);
> +               startbit = le32_to_cpu(ebitmap_start);
>
>                 if (startbit & (mapunit - 1)) {
>                         pr_err("SELinux: ebitmap start bit (%d) is "
> @@ -423,12 +425,12 @@ int ebitmap_read(struct ebitmap *e, void *fp)
>                         goto bad;
>                 }
>
> -               rc = next_entry(&map, fp, sizeof(u64));
> +               rc = next_entry(&mapbits, fp, sizeof(u64));
>                 if (rc < 0) {
>                         pr_err("SELinux: ebitmap: truncated map\n");
>                         goto bad;
>                 }
> -               map = le64_to_cpu(map);
> +               map = le64_to_cpu(mapbits);
>
>                 index = (startbit - n->startbit) / EBITMAP_UNIT_SIZE;
>                 while (map) {
> --
> 2.1.4
>


-- 
paul moore
www.paul-moore.com

Re: [PATCH RFC] selinux: provide __le variables explicitly

[Nicholas Mc Guire]

On Wed, May 08, 2019 at 05:47:32PM -0400, Paul Moore wrote:
> On Wed, May 8, 2019 at 2:27 AM Nicholas Mc Guire <hofrat@osadl.org> wrote:
> > While the endiannes is being handled properly sparse was unable to verify
> > this due to type inconsistency. So introduce an additional __le32
> > respectively _le64 variable to be passed to le32/64_to_cpu() to allow
> > sparse to verify proper typing. Note that this patch does not change
> > the generated binary on little-endian systems - on 32bit powerpc it
> > does change the binary.
> >
> > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > ---
> >
> > Problem located by an experimental coccinelle script to locate
> > patters that make sparse unhappy (false positives):
> >
> > sparse complaints on different architectures fixed by this patch are:
> >
> > ppc6xx_defconfig
> >   CHECK   security/selinux/ss/ebitmap.c
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> >
> > Little-endian systems:
> >
> > loongson3_defconfig
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> >
> > x86_64_defconfig
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> >
> > Patch was compile-tested with: x86_64_defconfig,loongson3_defconfig (both
> > little-endian) and ppc603_defconfig (big-endian).
> >
> > On little-endian systems the patch has no impact on the generated binary
> > (which is expected) but on the 32bit powerpc it does change the binary
> > which is not expected but since I'm not able to generate the .lst files
> > in security/selinux/ss/ due to the lack of a Makefile it is not clear
> > if this is an unexpected side-effect or due only to the introduction of
> > the additional variables. From my understanding the patch does not change
> > the program logic so if the code was correct on big-endian systems before
> > it should still be correct now.
> 
> This is a bit worrisome, but I tend to agree that this patch *should*
> be correct.  I'm thinking you're probably right in that the resulting
> binary difference could be due to the extra variable.  Have you tried
> any other big-endian arches?
>

just tried ppc64_defconfig + AUDIT=y, SECURITY=y, SECURITY_NETWORK=y, SECURITY_SELINUX=y

sparse will complain in the original version about:
  CHECK   security/selinux/ss/ebitmap.c
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64

which is the same as 32bit ppc - after the patch is applied that is resolved
and and the generated ebitmap.o files are binary identical.

I just had chosen ppc6xx_defconfig as my big-endian test-target as SELINUX
was on there by default so I assumed it would be the most reasonable
compile-test target.

thx!
hofrat
 
> > Patch is against 5.1 (localversion-next is next-20190506)
> >
> >  security/selinux/ss/ebitmap.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
> > index 8f624f8..09929fc 100644
> > --- a/security/selinux/ss/ebitmap.c
> > +++ b/security/selinux/ss/ebitmap.c
> > @@ -347,7 +347,9 @@ int ebitmap_read(struct ebitmap *e, void *fp)
> >  {
> >         struct ebitmap_node *n = NULL;
> >         u32 mapunit, count, startbit, index;
> > +       __le32 ebitmap_start;
> >         u64 map;
> > +       __le64 mapbits;
> >         __le32 buf[3];
> >         int rc, i;
> >
> > @@ -381,12 +383,12 @@ int ebitmap_read(struct ebitmap *e, void *fp)
> >                 goto bad;
> >
> >         for (i = 0; i < count; i++) {
> > -               rc = next_entry(&startbit, fp, sizeof(u32));
> > +               rc = next_entry(&ebitmap_start, fp, sizeof(u32));
> >                 if (rc < 0) {
> >                         pr_err("SELinux: ebitmap: truncated map\n");
> >                         goto bad;
> >                 }
> > -               startbit = le32_to_cpu(startbit);
> > +               startbit = le32_to_cpu(ebitmap_start);
> >
> >                 if (startbit & (mapunit - 1)) {
> >                         pr_err("SELinux: ebitmap start bit (%d) is "
> > @@ -423,12 +425,12 @@ int ebitmap_read(struct ebitmap *e, void *fp)
> >                         goto bad;
> >                 }
> >
> > -               rc = next_entry(&map, fp, sizeof(u64));
> > +               rc = next_entry(&mapbits, fp, sizeof(u64));
> >                 if (rc < 0) {
> >                         pr_err("SELinux: ebitmap: truncated map\n");
> >                         goto bad;
> >                 }
> > -               map = le64_to_cpu(map);
> > +               map = le64_to_cpu(mapbits);
> >
> >                 index = (startbit - n->startbit) / EBITMAP_UNIT_SIZE;
> >                 while (map) {
> > --
> > 2.1.4
> >
> 
> 
> -- 
> paul moore
> www.paul-moore.com

Re: [PATCH RFC] selinux: provide __le variables explicitly

[Paul Moore]

On Wed, May 8, 2019 at 8:14 PM Nicholas Mc Guire <der.herr@hofr.at> wrote:
> On Wed, May 08, 2019 at 05:47:32PM -0400, Paul Moore wrote:
> > On Wed, May 8, 2019 at 2:27 AM Nicholas Mc Guire <hofrat@osadl.org> wrote:
> > > While the endiannes is being handled properly sparse was unable to verify
> > > this due to type inconsistency. So introduce an additional __le32
> > > respectively _le64 variable to be passed to le32/64_to_cpu() to allow
> > > sparse to verify proper typing. Note that this patch does not change
> > > the generated binary on little-endian systems - on 32bit powerpc it
> > > does change the binary.
> > >
> > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > > ---
> > >
> > > Problem located by an experimental coccinelle script to locate
> > > patters that make sparse unhappy (false positives):
> > >
> > > sparse complaints on different architectures fixed by this patch are:
> > >
> > > ppc6xx_defconfig
> > >   CHECK   security/selinux/ss/ebitmap.c
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > >
> > > Little-endian systems:
> > >
> > > loongson3_defconfig
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > >
> > > x86_64_defconfig
> > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > >
> > > Patch was compile-tested with: x86_64_defconfig,loongson3_defconfig (both
> > > little-endian) and ppc603_defconfig (big-endian).
> > >
> > > On little-endian systems the patch has no impact on the generated binary
> > > (which is expected) but on the 32bit powerpc it does change the binary
> > > which is not expected but since I'm not able to generate the .lst files
> > > in security/selinux/ss/ due to the lack of a Makefile it is not clear
> > > if this is an unexpected side-effect or due only to the introduction of
> > > the additional variables. From my understanding the patch does not change
> > > the program logic so if the code was correct on big-endian systems before
> > > it should still be correct now.
> >
> > This is a bit worrisome, but I tend to agree that this patch *should*
> > be correct.  I'm thinking you're probably right in that the resulting
> > binary difference could be due to the extra variable.  Have you tried
> > any other big-endian arches?
> >
>
> just tried ppc64_defconfig + AUDIT=y, SECURITY=y, SECURITY_NETWORK=y, SECURITY_SELINUX=y
>
> sparse will complain in the original version about:
>   CHECK   security/selinux/ss/ebitmap.c
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
>
> which is the same as 32bit ppc - after the patch is applied that is resolved
> and and the generated ebitmap.o files are binary identical.
>
> I just had chosen ppc6xx_defconfig as my big-endian test-target as SELINUX
> was on there by default so I assumed it would be the most reasonable
> compile-test target.

Thanks.

I think this is probably safe to merge once the merge window closes.

-- 
paul moore
www.paul-moore.com

Re: [PATCH RFC] selinux: provide __le variables explicitly

[Paul Moore]

On Thu, May 9, 2019 at 3:40 PM Paul Moore <paul@paul-moore.com> wrote:
> On Wed, May 8, 2019 at 8:14 PM Nicholas Mc Guire <der.herr@hofr.at> wrote:
> > On Wed, May 08, 2019 at 05:47:32PM -0400, Paul Moore wrote:
> > > On Wed, May 8, 2019 at 2:27 AM Nicholas Mc Guire <hofrat@osadl.org> wrote:
> > > > While the endiannes is being handled properly sparse was unable to verify
> > > > this due to type inconsistency. So introduce an additional __le32
> > > > respectively _le64 variable to be passed to le32/64_to_cpu() to allow
> > > > sparse to verify proper typing. Note that this patch does not change
> > > > the generated binary on little-endian systems - on 32bit powerpc it
> > > > does change the binary.
> > > >
> > > > Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> > > > ---
> > > >
> > > > Problem located by an experimental coccinelle script to locate
> > > > patters that make sparse unhappy (false positives):
> > > >
> > > > sparse complaints on different architectures fixed by this patch are:
> > > >
> > > > ppc6xx_defconfig
> > > >   CHECK   security/selinux/ss/ebitmap.c
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > >
> > > > Little-endian systems:
> > > >
> > > > loongson3_defconfig
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > >
> > > > x86_64_defconfig
> > > > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > > > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > > >
> > > > Patch was compile-tested with: x86_64_defconfig,loongson3_defconfig (both
> > > > little-endian) and ppc603_defconfig (big-endian).
> > > >
> > > > On little-endian systems the patch has no impact on the generated binary
> > > > (which is expected) but on the 32bit powerpc it does change the binary
> > > > which is not expected but since I'm not able to generate the .lst files
> > > > in security/selinux/ss/ due to the lack of a Makefile it is not clear
> > > > if this is an unexpected side-effect or due only to the introduction of
> > > > the additional variables. From my understanding the patch does not change
> > > > the program logic so if the code was correct on big-endian systems before
> > > > it should still be correct now.
> > >
> > > This is a bit worrisome, but I tend to agree that this patch *should*
> > > be correct.  I'm thinking you're probably right in that the resulting
> > > binary difference could be due to the extra variable.  Have you tried
> > > any other big-endian arches?
> > >
> >
> > just tried ppc64_defconfig + AUDIT=y, SECURITY=y, SECURITY_NETWORK=y, SECURITY_SELINUX=y
> >
> > sparse will complain in the original version about:
> >   CHECK   security/selinux/ss/ebitmap.c
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:389:28: warning: cast to restricted __le32
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> > security/selinux/ss/ebitmap.c:431:23: warning: cast to restricted __le64
> >
> > which is the same as 32bit ppc - after the patch is applied that is resolved
> > and and the generated ebitmap.o files are binary identical.
> >
> > I just had chosen ppc6xx_defconfig as my big-endian test-target as SELINUX
> > was on there by default so I assumed it would be the most reasonable
> > compile-test target.
>
> Thanks.
>
> I think this is probably safe to merge once the merge window closes.

... and that time is now; merged into selinux/next.  Thanks.

/me crosses his fingers on this one

-- 
paul moore
www.paul-moore.com