mbox series

[v2,0/4] improve runtime loading of shared drivers

Message ID 20200622143337.562637-1-bruce.richardson@intel.com (mailing list archive)
Headers
Series improve runtime loading of shared drivers |

Message

Bruce Richardson June 22, 2020, 2:33 p.m. UTC
  This set includes a number of small improvements for handling the loading
of drivers at runtime using the EAL -d flag.

It limits the loading of files to only those files which end in .so, which
means that one can pass in the whole "drivers/" subfolder from a meson
build and not get an error when DPDK trys to load a .a file.

It also puts in some basic permission checking to ensure that no drivers
are loaded from a world-writable location on the filesystem, which would be
a potential security hole on a mis-configured system.

v2: rebased to fix errors on apply
    fixed one checkpatch issue.

Bruce Richardson (4):
  eal: remove unnecessary null-termination
  eal: only load shared libs from driver plugin directory
  eal: don't load drivers from insecure paths
  eal: cache last directory permissions checked

 lib/librte_eal/common/eal_common_options.c | 92 +++++++++++++++++++---
 1 file changed, 82 insertions(+), 10 deletions(-)
  

Comments

Thomas Monjalon July 2, 2020, 9:13 p.m. UTC | #1
22/06/2020 16:33, Bruce Richardson:
> This set includes a number of small improvements for handling the loading
> of drivers at runtime using the EAL -d flag.
> 
> It limits the loading of files to only those files which end in .so, which
> means that one can pass in the whole "drivers/" subfolder from a meson
> build and not get an error when DPDK trys to load a .a file.
> 
> It also puts in some basic permission checking to ensure that no drivers
> are loaded from a world-writable location on the filesystem, which would be
> a potential security hole on a mis-configured system.
> 
> v2: rebased to fix errors on apply
>     fixed one checkpatch issue.
> 
> Bruce Richardson (4):
>   eal: remove unnecessary null-termination
>   eal: only load shared libs from driver plugin directory
>   eal: don't load drivers from insecure paths
>   eal: cache last directory permissions checked

There is an error when running devtools/test-null.sh:

EAL: Error with realpath, No such file or directory
EAL: FATAL: Cannot init plugins
  
Thomas Monjalon July 2, 2020, 9:16 p.m. UTC | #2
22/06/2020 16:33, Bruce Richardson:
> Bruce Richardson (4):
>   eal: remove unnecessary null-termination

Maybe add scope of the change with "in plugin path" ?

>   eal: only load shared libs from driver plugin directory

I suggest: "eal: load only shared libraries from plugin directory"

>   eal: don't load drivers from insecure paths

I don't know why, I don't like titles starting with "don't".
I suggest: "eal: forbid plugin from insecure path"

>   eal: cache last directory permissions checked
  
Bruce Richardson July 3, 2020, 8:33 a.m. UTC | #3
On Thu, Jul 02, 2020 at 11:16:51PM +0200, Thomas Monjalon wrote:
> 22/06/2020 16:33, Bruce Richardson:
> > Bruce Richardson (4):
> >   eal: remove unnecessary null-termination
> 
> Maybe add scope of the change with "in plugin path" ?
> 
> >   eal: only load shared libs from driver plugin directory
> 
> I suggest: "eal: load only shared libraries from plugin directory"
> 
> >   eal: don't load drivers from insecure paths
> 
> I don't know why, I don't like titles starting with "don't".
> I suggest: "eal: forbid plugin from insecure path"
> 
> >   eal: cache last directory permissions checked
> 
Will adjust for v3
  
Bruce Richardson July 3, 2020, 10:25 a.m. UTC | #4
On Thu, Jul 02, 2020 at 11:13:02PM +0200, Thomas Monjalon wrote:
> 22/06/2020 16:33, Bruce Richardson:
> > This set includes a number of small improvements for handling the loading
> > of drivers at runtime using the EAL -d flag.
> > 
> > It limits the loading of files to only those files which end in .so, which
> > means that one can pass in the whole "drivers/" subfolder from a meson
> > build and not get an error when DPDK trys to load a .a file.
> > 
> > It also puts in some basic permission checking to ensure that no drivers
> > are loaded from a world-writable location on the filesystem, which would be
> > a potential security hole on a mis-configured system.
> > 
> > v2: rebased to fix errors on apply
> >     fixed one checkpatch issue.
> > 
> > Bruce Richardson (4):
> >   eal: remove unnecessary null-termination
> >   eal: only load shared libs from driver plugin directory
> >   eal: don't load drivers from insecure paths
> >   eal: cache last directory permissions checked
> 
> There is an error when running devtools/test-null.sh:
> 
> EAL: Error with realpath, No such file or directory
> EAL: FATAL: Cannot init plugins
> 
Yes, I missed the fact that we can load drivers without paths letting
dlopen search system directories. I think we can assume system dirs are
secure, and so can just skip any permission checks in case where we can't
get the realpath of the filename passed in.

Fixed in v3.