Re: [PATCH 1/3] libselinux: add missing glue code to grab errno in Python bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>
> > -----Original Message-----
> > From: selinux-owner@xxxxxxxxxxxxxxx [mailto:selinux-owner@xxxxxxxxxxxxxxx]
> > On Behalf Of Nicolas Iooss
> > Sent: Monday, April 13, 2020 11:15 AM
> > To: William Roberts <bill.c.roberts@xxxxxxxxx>
> > Cc: SElinux list <selinux@xxxxxxxxxxxxxxx>
> > Subject: Re: [PATCH 1/3] libselinux: add missing glue code to grab errno in Python
> > bindings
> >
> > On Mon, Apr 13, 2020 at 4:18 PM William Roberts <bill.c.roberts@xxxxxxxxx>
> > wrote:
> > >
> > > On Sun, Apr 12, 2020 at 3:12 AM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
> > > >
> > > > The Python bindings for libselinux expose functions such as
> > > > avc_has_perm(), get_ordered_context_list(), etc. When these
> > > > functions encounter an error, they set errno accordingly and return
> > > > a negative value. In order to get the value of errno from Python
> > > > code, it needs to be "forwarded" in a way. This is achieved by glue
> > > > code in selinuxswig_python_exception.i, which implement raising an
> > > > OSError exception from the value of errno.
> > > >
> > > > selinuxswig_python_exception.i was only generating glue code from
> > > > functions declared in selinux.h and not in other headers. Add other
> > > > headers.
> > > >
> > > > selinuxswig_python_exception.i is generated by "bash exception.sh".
> > > > Mark the fact that exception.sh is a Bash script by adding a
> > > > shebang. This makes "shellcheck" not warn about the Bash array which
> > > > is used to list header files.
> > > >
> > > > Signed-off-by: Nicolas Iooss <nicolas.iooss@xxxxxxx>
> > > > ---
> > > >  libselinux/src/exception.sh                   |  18 +-
> > > >  libselinux/src/selinuxswig_python_exception.i | 396
> > > > ++++++++++++++++++
> > > >  2 files changed, 412 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/libselinux/src/exception.sh
> > > > b/libselinux/src/exception.sh index 33ceef804af5..644c7a05ec54
> > > > 100755
> > > > --- a/libselinux/src/exception.sh
> > > > +++ b/libselinux/src/exception.sh
> > > > @@ -1,3 +1,5 @@
> > > > +#!/bin/bash
> > > > +
> > > >  function except() {
> > > >  case $1 in
> > > >      selinux_file_context_cmp) # ignore @@ -15,10 +17,22 @@ echo "
> > > >  ;;
> > > >  esac
> > > >  }
> > > > -if ! ${CC:-gcc} -x c -c -I../include -o temp.o - -aux-info temp.aux
> > > > < ../include/selinux/selinux.h
> > > > +
> > > > +# Make sure that selinux.h is included first in order not to depend
> > > > +on the order # in which "#include <selinux/selinux.h>" appears in other files.
> > > > +FILE_LIST=(
> > > > +    ../include/selinux/selinux.h
> > > > +    ../include/selinux/avc.h
> > > > +    ../include/selinux/context.h
> > > > +    ../include/selinux/get_context_list.h
> > > > +    ../include/selinux/get_default_type.h
> > > > +    ../include/selinux/label.h
> > > > +    ../include/selinux/restorecon.h
> > > > +)
> > > > +if ! cat "${FILE_LIST[@]}" | ${CC:-gcc} -x c -c -I../include -o
> > > > +temp.o - -aux-info temp.aux
> > > >  then
> > > >      # clang does not support -aux-info so fall back to gcc
> > > > -    gcc -x c -c -I../include -o temp.o - -aux-info temp.aux <
> > ../include/selinux/selinux.h
> > > > +    cat "${FILE_LIST[@]}" | gcc -x c -c -I../include -o temp.o -
> > > > + -aux-info temp.aux
> > > >  fi
> > > >  for i in `awk '/<stdin>.*extern int/ { print $6 }' temp.aux`; do
> > > > except $i ; done  rm -f -- temp.aux temp.o diff --git
> > > > a/libselinux/src/selinuxswig_python_exception.i
> > > > b/libselinux/src/selinuxswig_python_exception.i
> > > > index cf6582595ee7..9f1f86a5564d 100644
> > > > --- a/libselinux/src/selinuxswig_python_exception.i
> > > > +++ b/libselinux/src/selinuxswig_python_exception.i
> > > > @@ -952,3 +952,399 @@
> > > >    }
> > > >  }
> > > >
> > > > +
> > > > +%exception avc_sid_to_context {
> > > > +  $action
> > > > +  if (result < 0) {
> > > > +     PyErr_SetFromErrno(PyExc_OSError);
> > > > +     SWIG_fail;
> > > > +  }
> > > > +}
> > > > +
> > > > +
> > > > +%exception avc_sid_to_context_raw {
> > > > +  $action
> > > > +  if (result < 0) {
> > > > +     PyErr_SetFromErrno(PyExc_OSError);
> > > > +     SWIG_fail;
> > > > +  }
> > > > +}
> > [...]
> > >
> > > A few comments:
> > > - overall looks fine, builds and works as expected.
> > > - Why the double newline space on the exception swig file? The other
> > > .i files seem to do a single newline?
> > >   is their something I am missing with syntax?
> > > - I have the following whitespace warning:
> > > Applying: libselinux: add missing glue code to grab errno in Python
> > > bindings
> > > .git/rebase-apply/patch:444: new blank line at EOF.
> > > +
> > > warning: 1 line adds whitespace errors.
> >
> > The last two points are due to the way the file is generated, by exception.sh:
> >
> > echo "
> > %exception $1 {
> >   \$action
> >   if (result < 0) {
> >      PyErr_SetFromErrno(PyExc_OSError);
> >      SWIG_fail;
> >   }
> > }
> > "
> > ... this introduces blank lines both before and after each exception blocks. We
> > could remove the one after the block by using }" in the shell script. I will submit a
> > patch that does this once this patch is merged, as this makes the file cleaner.
>
> WFM. Thanks for fixing this, I used the python bindings eons ago and remember
> being frustrated I didn't get really good errors.
>
> Acked-by: William Roberts <william.c.roberts@xxxxxxxxx>

Merged:
https://github.com/SELinuxProject/selinux/pull/221



[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux