* [PATCH RFC] selinux: provide __le variables explicitly
@ 2019-05-08 6:21 Nicholas Mc Guire
2019-05-08 21:47 ` Paul Moore
0 siblings, 1 reply; 5+ messages in thread
From: Nicholas Mc Guire @ 2019-05-08 6:21 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Smalley, Eric Paris, peter enderborg, selinux,
linux-kernel, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH RFC] selinux: provide __le variables explicitly
2019-05-08 6:21 [PATCH RFC] selinux: provide __le variables explicitly Nicholas Mc Guire
@ 2019-05-08 21:47 ` Paul Moore
2019-05-09 0:13 ` Nicholas Mc Guire
0 siblings, 1 reply; 5+ messages in thread
From: Paul Moore @ 2019-05-08 21:47 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Stephen Smalley, Eric Paris, peter enderborg, selinux, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFC] selinux: provide __le variables explicitly
2019-05-08 21:47 ` Paul Moore
@ 2019-05-09 0:13 ` Nicholas Mc Guire
2019-05-09 19:40 ` Paul Moore
0 siblings, 1 reply; 5+ messages in thread
From: Nicholas Mc Guire @ 2019-05-09 0:13 UTC (permalink / raw)
To: Paul Moore
Cc: Nicholas Mc Guire, Stephen Smalley, Eric Paris, peter enderborg,
selinux, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFC] selinux: provide __le variables explicitly
2019-05-09 0:13 ` Nicholas Mc Guire
@ 2019-05-09 19:40 ` Paul Moore
2019-05-21 20:23 ` Paul Moore
0 siblings, 1 reply; 5+ messages in thread
From: Paul Moore @ 2019-05-09 19:40 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Nicholas Mc Guire, Stephen Smalley, Eric Paris, peter enderborg,
selinux, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFC] selinux: provide __le variables explicitly
2019-05-09 19:40 ` Paul Moore
@ 2019-05-21 20:23 ` Paul Moore
0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2019-05-21 20:23 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Nicholas Mc Guire, Stephen Smalley, Eric Paris, peter enderborg,
selinux, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-21 20:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-08 6:21 [PATCH RFC] selinux: provide __le variables explicitly Nicholas Mc Guire
2019-05-08 21:47 ` Paul Moore
2019-05-09 0:13 ` Nicholas Mc Guire
2019-05-09 19:40 ` Paul Moore
2019-05-21 20:23 ` Paul Moore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).