On Wed, May 8, 2019 at 8:14 PM Nicholas Mc Guire <der.herr@xxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx> > > > --- > > > > > > 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