Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220217142133.72205-1-cgzones@googlemail.com> <20220217142133.72205-2-cgzones@googlemail.com>
In-Reply-To: <20220217142133.72205-2-cgzones@googlemail.com>
From:   Paul Moore <paul@paul-moore.com>
Date:   Fri, 18 Feb 2022 11:01:10 -0500
Message-ID: <CAHC9VhQPZ3_dXvM9eqwsxZ2c+2mcdvNNJbtUNs_-9Md5F3P8vA@mail.gmail.com>
Subject: Re: [PATCH 3/5] selinux: use consistent pointer types for boolean arrays
To:     =?UTF-8?Q?Christian_G=C3=B6ttsche?= <cgzones@googlemail.com>
Cc:     selinux@vger.kernel.org,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Ondrej Mosnacek <omosnace@redhat.com>,
        James Morris <jamorris@linux.microsoft.com>,
        Austin Kim <austin.kim@lge.com>,
        Casey Schaufler <casey@schaufler-ca.com>,
        Jiapeng Chong <jiapeng.chong@linux.alibaba.com>,
        Yang Li <yang.lee@linux.alibaba.com>,
        linux-kernel@vger.kernel.org, llvm@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Feb 17, 2022 at 9:21 AM Christian G=C3=B6ttsche
<cgzones@googlemail.com> wrote:
>
> Use a consistent type of unsigned int* for boolean arrays, instead of
> using implicit casts to and from int*.
>
> Reported by sparse:
>
>     security/selinux/selinuxfs.c:1481:30: warning: incorrect type in assi=
gnment (different signedness)
>     security/selinux/selinuxfs.c:1481:30:    expected unsigned int *
>     security/selinux/selinuxfs.c:1481:30:    got int *[addressable] value=
s
>     security/selinux/selinuxfs.c:1398:48: warning: incorrect type in argu=
ment 3 (different signedness)
>     security/selinux/selinuxfs.c:1398:48:    expected int *values
>     security/selinux/selinuxfs.c:1398:48:    got unsigned int *bool_pendi=
ng_values
>
> Signed-off-by: Christian G=C3=B6ttsche <cgzones@googlemail.com>
>
> ---
> A more invasive change would be to change all boolean arrays to bool*.

I think that might be a worthwhile change, although that can happen at
a later date.

A quick general comment: please try to stick to 80-char long lines.  I
realize Linus/checkpatch.pl has started to allow longer lines but I
would still like SELinux to try and keep to 80-chars or under.

> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/service=
s.c
> index 6901dc07680d..7865926962ab 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -3175,7 +3175,8 @@ int security_get_bool_value(struct selinux_state *s=
tate,
>  static int security_preserve_bools(struct selinux_policy *oldpolicy,
>                                 struct selinux_policy *newpolicy)
>  {
> -       int rc, *bvalues =3D NULL;
> +       int rc;
> +       unsigned int *bvalues =3D NULL;

Doesn't this cause a type mismatch (unsigned int vs int) when an entry
from bvalues[] is assigned to cond_bool_datum::state later in the
security_preserve_bools() function?

--=20
paul-moore.com