lkml.org 
[lkml]   [2019]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH RFC] selinux: provide __le variables explicitly
    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

    \
     
     \ /
      Last update: 2019-05-08 23:48    [W:3.891 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site