patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH] doc: add capability to access physical addresses
@ 2023-01-14 22:58 Dmitry Kozlyuk
  2023-01-15  2:27 ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Kozlyuk @ 2023-01-14 22:58 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Kozlyuk, stable, Boris Ouretskey, Isaac Boukris, Bruce Richardson

CAP_DAC_OVERRIDE capability is required to access /proc/self/pagemap,
but it was missing from the Linux guide, causing issues for users.

Fixes: 979bb5d493fb ("doc: add more instructions for running as non-root")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Reported-by: Boris Ouretskey <borisusun@gmail.com>
Reported-by: Isaac Boukris <iboukris@gmail.com>
---
Mail threads:
* https://inbox.dpdk.org/users/CAG4AAQ21hn_Ogi0-gfUjXVhtWU19Bk634BT6Ue1bpYZjRXFJjg@mail.gmail.com
* https://inbox.dpdk.org/users/CAC-fF8ShK_n1dhPK2KwV7nRx7535simb=fN77yrZR1+fV2j28A@mail.gmail.com

 doc/guides/linux_gsg/enable_func.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/linux_gsg/enable_func.rst b/doc/guides/linux_gsg/enable_func.rst
index 829084d80e..b0e123cb7e 100644
--- a/doc/guides/linux_gsg/enable_func.rst
+++ b/doc/guides/linux_gsg/enable_func.rst
@@ -55,12 +55,12 @@ Refer to the `documentation <https://www.kernel.org/doc/Documentation/vm/hugetlb
 If the driver requires using physical addresses (PA),
 the executable file must be granted additional capabilities:
 
-* ``SYS_ADMIN`` to read ``/proc/self/pagemaps``
+* ``CAP_DAC_OVERRIDE`` and ``SYS_ADMIN`` to read ``/proc/self/pagemaps``
 * ``IPC_LOCK`` to lock hugepages in memory
 
 .. code-block:: console
 
-   setcap cap_ipc_lock,cap_sys_admin+ep <executable>
+   setcap cap_dac_override,cap_ipc_lock,cap_sys_admin+ep <executable>
 
 If physical addresses are not accessible,
 the following message will appear during EAL initialization::
-- 
2.38.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] doc: add capability to access physical addresses
  2023-01-14 22:58 [PATCH] doc: add capability to access physical addresses Dmitry Kozlyuk
@ 2023-01-15  2:27 ` Stephen Hemminger
  2023-01-15  6:20   ` Isaac Boukris
  2023-01-15 12:46   ` Dmitry Kozlyuk
  0 siblings, 2 replies; 7+ messages in thread
From: Stephen Hemminger @ 2023-01-15  2:27 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, stable, Boris Ouretskey, Isaac Boukris, Bruce Richardson

On Sun, 15 Jan 2023 01:58:02 +0300
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:

> CAP_DAC_OVERRIDE capability is required to access /proc/self/pagemap,
> but it was missing from the Linux guide, causing issues for users.
> 
> Fixes: 979bb5d493fb ("doc: add more instructions for running as non-root")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Reported-by: Boris Ouretskey <borisusun@gmail.com>
> Reported-by: Isaac Boukris <iboukris@gmail.com>

DAC_OVERRIDE is like having the master key. It opens all doors
and if so, running as non-root really doesn't matter that much.

Ideally, a finer grain permission could be used.
Recommending this to users seems wrong.

According proc.5 man page.


       /proc/[pid]/pagemap (since Linux 2.6.25)
              This file shows the mapping of each of the process's
              virtual pages into physical page frames or swap area.
...
              Permission to access this file is governed by a ptrace
              access mode PTRACE_MODE_READ_FSCREDS check; see ptrace(2).

Which distro is this? What security module are you using.
For example, on Debian (kernel 5.17) running as non-root it is possible to read pagemap.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] doc: add capability to access physical addresses
  2023-01-15  2:27 ` Stephen Hemminger
@ 2023-01-15  6:20   ` Isaac Boukris
  2023-01-15 12:46   ` Dmitry Kozlyuk
  1 sibling, 0 replies; 7+ messages in thread
From: Isaac Boukris @ 2023-01-15  6:20 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Dmitry Kozlyuk, dev, stable, Boris Ouretskey, Bruce Richardson

On Sun, Jan 15, 2023 at 4:27 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Sun, 15 Jan 2023 01:58:02 +0300
> Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> > CAP_DAC_OVERRIDE capability is required to access /proc/self/pagemap,
> > but it was missing from the Linux guide, causing issues for users.
> >
> > Fixes: 979bb5d493fb ("doc: add more instructions for running as non-root")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > Reported-by: Boris Ouretskey <borisusun@gmail.com>
> > Reported-by: Isaac Boukris <iboukris@gmail.com>
>
> DAC_OVERRIDE is like having the master key. It opens all doors
> and if so, running as non-root really doesn't matter that much.

The cap_sys_admin also seems heavy but I guessed it is still better
than full root.

> Ideally, a finer grain permission could be used.
> Recommending this to users seems wrong.
>
> According proc.5 man page.
>
>
>        /proc/[pid]/pagemap (since Linux 2.6.25)
>               This file shows the mapping of each of the process's
>               virtual pages into physical page frames or swap area.
> ...
>               Permission to access this file is governed by a ptrace
>               access mode PTRACE_MODE_READ_FSCREDS check; see ptrace(2).
>
> Which distro is this? What security module are you using.
> For example, on Debian (kernel 5.17) running as non-root it is possible to read pagemap.

I tested on fedora (but also on Rocky8 older kernel): uname -a
Linux localhost.localdomain 6.0.17-200.fc36.x86_64 #1 SMP
PREEMPT_DYNAMIC Wed Jan 4 16:00:03 UTC 2023 x86_64 x86_64 x86_64
GNU/Linux

It can be shown by running the 'pagemap.c' demo code from
https://bugs.centos.org/view.php?id=17176 which hinted me to adding
DAC_OVERRIDE.

The strange thing is that running it without any capabilities allows
you to read the file but give the leading zeros, upon adding
cap_ipc_lock,cap_sys_admin you get a read error and only adding
cap_dac_override lets it run successfully.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] doc: add capability to access physical addresses
  2023-01-15  2:27 ` Stephen Hemminger
  2023-01-15  6:20   ` Isaac Boukris
@ 2023-01-15 12:46   ` Dmitry Kozlyuk
  2023-01-15 13:30     ` Isaac Boukris
  1 sibling, 1 reply; 7+ messages in thread
From: Dmitry Kozlyuk @ 2023-01-15 12:46 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, stable, Boris Ouretskey, Isaac Boukris, Bruce Richardson

2023-01-14 18:27 (UTC-0800), Stephen Hemminger:
> DAC_OVERRIDE is like having the master key. It opens all doors
> and if so, running as non-root really doesn't matter that much.
> 
> Ideally, a finer grain permission could be used.
> Recommending this to users seems wrong.

According to my tests, DAC_READ_SEARCH can be used instead of DAC_OVERRIDE.
It seems slightly better, because it doesn't bypass write permission checks.
Although I agree with Isaac that SYS_ADMIN is already very powerful,
and remember that the final goal is to perform unrestricted DMA.
Boris, Isaac, is DAC_READ_SEARCH sufficient on your systems?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] doc: add capability to access physical addresses
  2023-01-15 12:46   ` Dmitry Kozlyuk
@ 2023-01-15 13:30     ` Isaac Boukris
  0 siblings, 0 replies; 7+ messages in thread
From: Isaac Boukris @ 2023-01-15 13:30 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: Stephen Hemminger, dev, stable, Boris Ouretskey, Bruce Richardson

Hi,

On Sun, Jan 15, 2023 at 2:46 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> 2023-01-14 18:27 (UTC-0800), Stephen Hemminger:
> > DAC_OVERRIDE is like having the master key. It opens all doors
> > and if so, running as non-root really doesn't matter that much.
> >
> > Ideally, a finer grain permission could be used.
> > Recommending this to users seems wrong.
>
> According to my tests, DAC_READ_SEARCH can be used instead of DAC_OVERRIDE.
> It seems slightly better, because it doesn't bypass write permission checks.
> Although I agree with Isaac that SYS_ADMIN is already very powerful,
> and remember that the final goal is to perform unrestricted DMA.
> Boris, Isaac, is DAC_READ_SEARCH sufficient on your systems?

Yes, I can confirm that DAC_READ_SEARCH works fine on my system as well. Thanks!

[admin@localhost ~]$ getcap /usr/bin/dpdk-testpmd
/usr/bin/dpdk-testpmd cap_dac_read_search,cap_ipc_lock,cap_sys_admin=ep
[admin@localhost ~]$ dpdk-testpmd -l 2,3,4 -a 000:0b:00.0 --huge-dir
/dev/hugepages/ --iova-mode pa
EAL: Detected CPU lcores: 8
EAL: Detected NUMA nodes: 1
EAL: Detected shared linkage of DPDK
EAL: Multi-process socket /tmp/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: No available 1048576 kB hugepages reported
TELEMETRY: No legacy callbacks, legacy socket not created
testpmd: No probed ethernet devices
testpmd: create a new mbuf pool <mb_pool_0>: n=163456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
Done

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] doc: add capability to access physical addresses
  2023-01-19 21:24 Dmitry Kozlyuk
@ 2023-03-28 19:19 ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2023-03-28 19:19 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, stable, stable, Boris Ouretskey, Isaac Boukris, Bruce Richardson

19/01/2023 22:24, Dmitry Kozlyuk:
> DAC_READ_SEARCH or DAC_OVERRIDE capability is required to access
> /proc/self/pagemap, but the Linux guide mentioned neither one.
> Recommend DAC_READ_SEARCH as less impactful.
> 
> Fixes: 979bb5d493fb ("doc: add more instructions for running as non-root")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Reported-by: Boris Ouretskey <borisusun@gmail.com>
> Reported-by: Isaac Boukris <iboukris@gmail.com>

Applied, thanks.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] doc: add capability to access physical addresses
@ 2023-01-19 21:24 Dmitry Kozlyuk
  2023-03-28 19:19 ` Thomas Monjalon
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Kozlyuk @ 2023-01-19 21:24 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Kozlyuk, stable, Boris Ouretskey, Isaac Boukris, Bruce Richardson

DAC_READ_SEARCH or DAC_OVERRIDE capability is required to access
/proc/self/pagemap, but the Linux guide mentioned neither one.
Recommend DAC_READ_SEARCH as less impactful.

Fixes: 979bb5d493fb ("doc: add more instructions for running as non-root")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Reported-by: Boris Ouretskey <borisusun@gmail.com>
Reported-by: Isaac Boukris <iboukris@gmail.com>
---
 doc/guides/linux_gsg/enable_func.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/linux_gsg/enable_func.rst b/doc/guides/linux_gsg/enable_func.rst
index 829084d80e..2344d97403 100644
--- a/doc/guides/linux_gsg/enable_func.rst
+++ b/doc/guides/linux_gsg/enable_func.rst
@@ -55,12 +55,12 @@ Refer to the `documentation <https://www.kernel.org/doc/Documentation/vm/hugetlb
 If the driver requires using physical addresses (PA),
 the executable file must be granted additional capabilities:
 
-* ``SYS_ADMIN`` to read ``/proc/self/pagemaps``
+* ``DAC_READ_SEARCH`` and ``SYS_ADMIN`` to read ``/proc/self/pagemaps``
 * ``IPC_LOCK`` to lock hugepages in memory
 
 .. code-block:: console
 
-   setcap cap_ipc_lock,cap_sys_admin+ep <executable>
+   setcap cap_dac_read_search,cap_ipc_lock,cap_sys_admin+ep <executable>
 
 If physical addresses are not accessible,
 the following message will appear during EAL initialization::
-- 
2.38.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-03-28 19:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-14 22:58 [PATCH] doc: add capability to access physical addresses Dmitry Kozlyuk
2023-01-15  2:27 ` Stephen Hemminger
2023-01-15  6:20   ` Isaac Boukris
2023-01-15 12:46   ` Dmitry Kozlyuk
2023-01-15 13:30     ` Isaac Boukris
2023-01-19 21:24 Dmitry Kozlyuk
2023-03-28 19:19 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).