Hi,
From: Masatake YAMATO <[email protected]>
Subject: Re: [LTP] [PATCH 2/3] Proc01: Fix for PPC64 and Support
SELinux-enabled Environment v2
Date: Thu, 12 Feb 2009 21:09:13 +0900 (JST)
> Sorry to be late.
>
>> > Index: testcases/kernel/fs/proc/proc01.c
>> > ===================================================================
>> > RCS file: /cvsroot/ltp/ltp/testcases/kernel/fs/proc/proc01.c,v
>> > retrieving revision 1.13
>> > diff -u -u -r1.13 proc01.c
>> > --- testcases/kernel/fs/proc/proc01.c 5 Feb 2009 11:20:49 -0000
>> > 1.13
>> > +++ testcases/kernel/fs/proc/proc01.c 6 Feb 2009 06:38:22 -0000
>> > @@ -107,19 +107,12 @@
>> >
>> > /* If a particular LSM is enabled, it is expected that some entries can
>> > be read successfully. */
>> > -#ifdef HAVE_SELINUX_SELINUX_H
>> > const char lsm_should_work[][PATH_MAX] =
>> > {
>> > "/proc/self/attr/*",
>> > "/proc/self/task/[0-9]*/attr/*",
>> > ""
>> > };
>> > -#else
>> > -const char lsm_should_work[][PATH_MAX] =
>> > - {
>> > - ""
>> > - };
>> > -#endif
>> >
>>
>> No, the above #ifdef statement is intended to make things easier for
>> other LSMs to add their specific entries there. The empty
>> lsm_should_work in the case that none of LSMs is found just to avoid a
>> compilation warning when the code is walking the array below,
>>
>> if (is_lsm_enabled())
>> {
>> for (i = 0; lsm_should_work[i][0] != '\0'; i++)
>> {
>> if (!strcmp(obj, lsm_should_work[i])
>> || !fnmatch(lsm_should_work[i], obj, FNM_PATHNAME))
>> return 0;
>> }
>> }
>
> I'm wrong. I misunderstood your code. I should not touch the array.
> It seems that I did not understand what I would like to do.
>
>> > /* Known files that does not honor O_NONBLOCK, so they will hang
>> > the test while being read. */
>> > @@ -132,7 +125,7 @@
>> > /* Check if a particular LSM is enabled. */
>> > int is_lsm_enabled(void)
>> > {
>> > -#ifdef HAVE_SELINUX_SELINUX_H
>> > +#if defined(HAVE_SELINUX_SELINUX_H) && defined(HAVE_LIBSELINUX)
>>
>> Is it possible to combine those two macros together if both headers and
>> libraries are detected? I can't think of any use case that you want to
>> try them separately.
>
> In this patch I'd like to handle the following two odd situations:
>
> <1> selinux.h is available, but libselinux is not.
> <2> libselinux is available, but selinux.h is not.
>
> My proposals to avoid *compile time error*:
>
> Under <1> let is_lsm_enabled return 0.
> Under <2> provide the declaration for is_selinux_enabled.
>
> Could you review again?
>
NAck. Please see comments below.
>
>
> Signed-off-by: Masatake YAMATO <[email protected]>
>
> diff -ruN old/m4/ltp-common.m4 new/m4/ltp-common.m4
> --- old/m4/ltp-common.m4 1970-01-01 09:00:00.000000000 +0900
> +++ new/m4/ltp-common.m4 2009-02-06 15:35:23.000000000 +0900
> @@ -0,0 +1,35 @@
> +dnl
> +dnl Copyright (c) Red Hat Inc., 2009
> +dnl
> +dnl This program is free software; you can redistribute it and/or
> +dnl modify it under the terms of the GNU General Public License as
> +dnl published by the Free Software Foundation; either version 2 of
> +dnl the License, or (at your option) any later version.
> +dnl
> +dnl This program is distributed in the hope that it will be useful,
> +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
> +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> +dnl the GNU General Public License for more details.
> +dnl
> +dnl You should have received a copy of the GNU General Public License
> +dnl along with this program; if not, write to the Free Software
> +dnl Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> +dnl USA
> +dnl
> +dnl Author: Masatake YAMATO <[email protected]>
> +dnl
> +
> +# LTP_CHECK_LIB(/LIBRARY/,/FUNCTIONS/,[/OTHER-LIBRARIES/])
> +# --------------------------------------------------
> +# LTP_CHECK_LIB works like AC_CHECK_LIB.
> +# But it is customized for LTP.
> +#
> +# 1. LIBS is not updated even if /FUNCTION/ is found in /LIBRARY/.
> +# 2. Instead of LIBS, /LIBRARY/_LIBS is set.
> +# 3. LIBS_/LIBRARY/ is passed to AC_SUBST.
> +#
> +AC_DEFUN([LTP_CHECK_LIB],LIBRARY_LIBS
> +[AH_TEMPLATE(AS_TR_CPP([HAVE_LIB$1]),
> +[Define to 1 if you have the `$1' library (-l$1).])
> +AC_CHECK_LIB($1,$2,[AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_LIB$1))
> AS_TR_CPP([$1_LIBS])="-l$1 $3"],,$3)
> +AC_SUBST(AS_TR_CPP([$1_LIBS]))])
>
With or without libselinux-devel, the messages from ./configure script
does not look good.
checking selinux/selinux.h usability... no
checking selinux/selinux.h presence... no
checking for selinux/selinux.h... no
./configure: line 5983: LIBRARY_LIBS: command not found
checking for is_selinux_enabled in -lselinux... no
configure: creating ./config.status
checking for selinux/selinux.h... yes
./configure: line 5983: LIBRARY_LIBS: command not found
checking for is_selinux_enabled in -lselinux... yes
configure: creating ./config.status
config.status: creating config.mk
> Index: m4/ltp-selinux.m4
> ===================================================================
> RCS file: /cvsroot/ltp/ltp/m4/ltp-selinux.m4,v
> retrieving revision 1.1
> diff -u -r1.1 ltp-selinux.m4
> --- m4/ltp-selinux.m4 5 Feb 2009 11:18:58 -0000 1.1
> +++ m4/ltp-selinux.m4 12 Feb 2009 11:56:20 -0000
> @@ -22,8 +22,6 @@
> dnl
> AC_DEFUN([LTP_CHECK_SELINUX],
> [dnl
> -AC_CHECK_HEADERS(selinux/selinux.h,[
> - SELINUX_LIBS="-lselinux"],[
> - SELINUX_LIBS=""])
> -AC_SUBST(SELINUX_LIBS)
> +AC_CHECK_HEADERS(selinux/selinux.h)
> +LTP_CHECK_LIB(selinux,is_selinux_enabled)
> ])
It is unnecessary to have two separate macros HAVE_SELINUX_SELINUX_H and
HAVE_LIBSELINUX here, since you don't want to treat those two situation
differently, do you?
<1> selinux.h is available, but libselinux is not.
<2> libselinux is available, but selinux.h is not.
How about to have one here like HAVE_LIBSELINX_DEVEL?
if HAVE_SELINUX_SELINUX_H && HAVE_LIBSELINUX; then
HAVE_LIBSELINUX_DEVEL = 1
else
HAVE_LIBSELINUX_DEVEL = 0
fi
In either of those situations, the test just let is_lsm_enabled() return
0, and don't call SELinux specific is_selinux_enabled() at all.
> Index: testcases/kernel/fs/proc/proc01.c
> ===================================================================
> RCS file: /cvsroot/ltp/ltp/testcases/kernel/fs/proc/proc01.c,v
> retrieving revision 1.13
> diff -u -r1.13 proc01.c
> --- testcases/kernel/fs/proc/proc01.c 5 Feb 2009 11:20:49 -0000 1.13
> +++ testcases/kernel/fs/proc/proc01.c 12 Feb 2009 11:56:25 -0000
> @@ -39,8 +39,14 @@
> #include <fcntl.h>
> #include <fnmatch.h>
>
> -#ifdef HAVE_SELINUX_SELINUX_H
> +#if defined(HAVE_SELINUX_SELINUX_H)
> #include <selinux/selinux.h>
> +#else
> +# if defined(HAVE_LIBSELINUX)
> + extern int is_selinux_enabled (void);
> +# else
> + int is_selinux_enabled (void) { return 0; }
> +# endif
> #endif
>
It is probably better to merge the alternative definition of
is_selinux_enabled() to the is_lsm_enabled() below, so the code is more
maintainable, and we set a better example for the people to add the next
LSM specific code in the future.
#ifdef HAVE_LIBSELINUX_DEVEL
#include <selinux/selinux.h>
#endif
...
int is_lsm_enabled(void)
{
#ifdef HAVE_LIBSELINUX_DEVEL
return is_selinux_enabled();
#else
return 0;
#endif
}
CAI Qian
> #include "test.h"
> @@ -132,11 +138,7 @@
> /* Check if a particular LSM is enabled. */
> int is_lsm_enabled(void)
> {
> -#ifdef HAVE_SELINUX_SELINUX_H
> return is_selinux_enabled();
> -#else
> - return 0;
> -#endif
> }
>
> /* Verify expected failures, and then let the test to continue. */
------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
