[PATCH] doc: add capability to access physical addresses

[PATCH] doc: add capability to access physical addresses

[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>
---
 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

Re: [PATCH] doc: add capability to access physical addresses

[Thomas Monjalon]

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.

Re: [PATCH] doc: add capability to access physical addresses

[Isaac Boukris]

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

Re: [PATCH] doc: add capability to access physical addresses

[Dmitry Kozlyuk]

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?

Re: [PATCH] doc: add capability to access physical addresses

[Isaac Boukris]

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.

Re: [PATCH] doc: add capability to access physical addresses

[Stephen Hemminger]

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.

[PATCH] doc: add capability to access physical addresses

[Dmitry Kozlyuk]

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