Re: [ovs-dev] [PATCH v3] dpdk: Allow retaining CAP_SYS_RAWIO privileges
On Tue, Mar 14, 2023 at 10:46:02AM -0400, Aaron Conole wrote:
> Flavio Leitner writes:
>
> > On Wed, Mar 08, 2023 at 05:37:11PM -0500, Aaron Conole wrote:
> >> Open vSwitch generally tries to let the underlying operating system
> >> managed the low level details of hardware, for example DMA mapping,
> >> bus arbitration, etc. However, when using DPDK, the underlying
> >> operating system yields control of many of these details to userspace
> >> for management.
> >>
> >> In the case of some DPDK port drivers, configuring rte_flow or even
> >> allocating resources may require access to iopl/ioperm calls, which
> >> are guarded by the CAP_SYS_RAWIO privilege on linux systems. These
> >> calls are dangerous, and can allow a process to completely compromise
> >> a system. However, they are needed in the case of some userspace
> >> driver code which manages the hardware (for example, the mlx
> >> implementation of backend support for rte_flow).
> >>
> >> Here, we create an opt-in flag passed to the command line to allow
> >> this access. We need to do this before ever accessing the database,
> >> because we want to drop all privileges asap, and cannot wait for
> >> a connection to the database to be established and functional before
> >> dropping. There may be distribution specific ways to do capability
> >> management as well (using for example, systemd), but they are not
> >> as universal to the vswitchd as a flag.
> >>
> >> Reviewed-by: Simon Horman
> >> Signed-off-by: Aaron Conole
> >> ---
> >> v1->v2: update daemon-windows for daemon_become_new_user
> >>
> >> v2->v3: update daemon-windows for daemon_start
> >> change log messages to be clearer
> >> update the manpage to provide example of why
> >> one would want the flag
> >>
> >> NEWS | 4
> >> lib/daemon-unix.c | 29 +
> >> lib/daemon-windows.c | 6 --
> >> lib/daemon.c | 2 +-
> >> lib/daemon.h | 4 ++--
> >> ovsdb/ovsdb-client.c | 6 +++---
> >> ovsdb/ovsdb-server.c | 4 ++--
> >> tests/test-netflow.c | 2 +-
> >> tests/test-sflow.c | 2 +-
> >> tests/test-unixctl.c | 2 +-
> >> utilities/ovs-ofctl.c | 4 ++--
> >> utilities/ovs-testcontroller.c | 4 ++--
> >> vswitchd/ovs-vswitchd.8.in | 9 +
> >> vswitchd/ovs-vswitchd.c| 11 ++-
> >> 14 files changed, 63 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/NEWS b/NEWS
> >> index 85b3496214..65f35dcdd5 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -10,6 +10,10 @@ Post-v3.1.0
> >> in order to create OVSDB sockets with access mode of 0770.
> >> - QoS:
> >> * Added new configuration option 'jitter' for a linux-netem QoS type.
> >> + - DPDK:
> >> + * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
> >> + with the --hw-rawio-access command line option. This allows the
> >> + process extra privileges when mapping physical interconnect memory.
> >>
> >>
> >> v3.1.0 - 16 Feb 2023
> >> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> >> index 1a7ba427d7..dd839015ab 100644
> >> --- a/lib/daemon-unix.c
> >> +++ b/lib/daemon-unix.c
> >> @@ -88,7 +88,8 @@ static bool switch_user = false;
> >> static uid_t uid;
> >> static gid_t gid;
> >> static char *user = NULL;
> >> -static void daemon_become_new_user__(bool access_datapath);
> >> +static void daemon_become_new_user__(bool access_datapath,
> >> + bool access_hardware_ports);
> >>
> >> static void check_already_running(void);
> >> static int lock_pidfile(FILE *, int command);
> >> @@ -443,13 +444,13 @@ monitor_daemon(pid_t daemon_pid)
> >> * daemonize_complete()) or that it failed to start up (by exiting with a
> >> * nonzero exit code). */
> >> void
> >> -daemonize_start(bool access_datapath)
> >> +daemonize_start(bool access_datapath, bool access_hardware_ports)
> >> {
> >> assert_single_threaded();
> >> daemonize_fd = -1;
> >>
> >> if (switch_user) {
> >> -daemon_become_new_user__(access_datapath);
> >> +daemon_become_new_user__(access_datapath, access_hardware_ports);
> >> switch_user = false;
> >> }
> >>
> >> @@ -807,7 +808,8 @@ daemon_become_new_user_unix(void)
> >> /* Linux specific implementation of daemon_become_new_user()
> >> * using libcap-ng. */
> >> static void
> >> -daemon_become_new_user_linux(bool access_datapath OVS_UNUSED)
> >> +daemon_become_new_user_linux(bool access_datapath OVS_UNUSED,
> >> + bool access_hardware_ports OVS_UNUSED)
> >> {
> >> #if defined __linux__ && HAVE_LIBCAPNG
> >> int ret;
> >> @@ -827,6 +829,16 @@ daemon_become_new_user_linux(bool access_datapath
> >> OVS_UNUSED)
> >> ret = capng_update(CAPNG_ADD, cap_sets, CAP_NET_ADMIN)
> >>
Re: [ovs-dev] [PATCH v3] dpdk: Allow retaining CAP_SYS_RAWIO privileges
Flavio Leitner writes:
> On Wed, Mar 08, 2023 at 05:37:11PM -0500, Aaron Conole wrote:
>> Open vSwitch generally tries to let the underlying operating system
>> managed the low level details of hardware, for example DMA mapping,
>> bus arbitration, etc. However, when using DPDK, the underlying
>> operating system yields control of many of these details to userspace
>> for management.
>>
>> In the case of some DPDK port drivers, configuring rte_flow or even
>> allocating resources may require access to iopl/ioperm calls, which
>> are guarded by the CAP_SYS_RAWIO privilege on linux systems. These
>> calls are dangerous, and can allow a process to completely compromise
>> a system. However, they are needed in the case of some userspace
>> driver code which manages the hardware (for example, the mlx
>> implementation of backend support for rte_flow).
>>
>> Here, we create an opt-in flag passed to the command line to allow
>> this access. We need to do this before ever accessing the database,
>> because we want to drop all privileges asap, and cannot wait for
>> a connection to the database to be established and functional before
>> dropping. There may be distribution specific ways to do capability
>> management as well (using for example, systemd), but they are not
>> as universal to the vswitchd as a flag.
>>
>> Reviewed-by: Simon Horman
>> Signed-off-by: Aaron Conole
>> ---
>> v1->v2: update daemon-windows for daemon_become_new_user
>>
>> v2->v3: update daemon-windows for daemon_start
>> change log messages to be clearer
>> update the manpage to provide example of why
>> one would want the flag
>>
>> NEWS | 4
>> lib/daemon-unix.c | 29 +
>> lib/daemon-windows.c | 6 --
>> lib/daemon.c | 2 +-
>> lib/daemon.h | 4 ++--
>> ovsdb/ovsdb-client.c | 6 +++---
>> ovsdb/ovsdb-server.c | 4 ++--
>> tests/test-netflow.c | 2 +-
>> tests/test-sflow.c | 2 +-
>> tests/test-unixctl.c | 2 +-
>> utilities/ovs-ofctl.c | 4 ++--
>> utilities/ovs-testcontroller.c | 4 ++--
>> vswitchd/ovs-vswitchd.8.in | 9 +
>> vswitchd/ovs-vswitchd.c| 11 ++-
>> 14 files changed, 63 insertions(+), 26 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 85b3496214..65f35dcdd5 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -10,6 +10,10 @@ Post-v3.1.0
>> in order to create OVSDB sockets with access mode of 0770.
>> - QoS:
>> * Added new configuration option 'jitter' for a linux-netem QoS type.
>> + - DPDK:
>> + * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
>> + with the --hw-rawio-access command line option. This allows the
>> + process extra privileges when mapping physical interconnect memory.
>>
>>
>> v3.1.0 - 16 Feb 2023
>> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
>> index 1a7ba427d7..dd839015ab 100644
>> --- a/lib/daemon-unix.c
>> +++ b/lib/daemon-unix.c
>> @@ -88,7 +88,8 @@ static bool switch_user = false;
>> static uid_t uid;
>> static gid_t gid;
>> static char *user = NULL;
>> -static void daemon_become_new_user__(bool access_datapath);
>> +static void daemon_become_new_user__(bool access_datapath,
>> + bool access_hardware_ports);
>>
>> static void check_already_running(void);
>> static int lock_pidfile(FILE *, int command);
>> @@ -443,13 +444,13 @@ monitor_daemon(pid_t daemon_pid)
>> * daemonize_complete()) or that it failed to start up (by exiting with a
>> * nonzero exit code). */
>> void
>> -daemonize_start(bool access_datapath)
>> +daemonize_start(bool access_datapath, bool access_hardware_ports)
>> {
>> assert_single_threaded();
>> daemonize_fd = -1;
>>
>> if (switch_user) {
>> -daemon_become_new_user__(access_datapath);
>> +daemon_become_new_user__(access_datapath, access_hardware_ports);
>> switch_user = false;
>> }
>>
>> @@ -807,7 +808,8 @@ daemon_become_new_user_unix(void)
>> /* Linux specific implementation of daemon_become_new_user()
>> * using libcap-ng. */
>> static void
>> -daemon_become_new_user_linux(bool access_datapath OVS_UNUSED)
>> +daemon_become_new_user_linux(bool access_datapath OVS_UNUSED,
>> + bool access_hardware_ports OVS_UNUSED)
>> {
>> #if defined __linux__ && HAVE_LIBCAPNG
>> int ret;
>> @@ -827,6 +829,16 @@ daemon_become_new_user_linux(bool access_datapath
>> OVS_UNUSED)
>> ret = capng_update(CAPNG_ADD, cap_sets, CAP_NET_ADMIN)
>>|| capng_update(CAPNG_ADD, cap_sets, CAP_NET_RAW)
>>|| capng_update(CAPNG_ADD, cap_sets,
>> CAP_NET_BROADCAST);
>> +#ifdef DPDK_NETDEV
>> +if (access_hardware_ports && !ret) {
>> +ret = capng_update(CAPNG_ADD,
Re: [ovs-dev] [PATCH v3] dpdk: Allow retaining CAP_SYS_RAWIO privileges
On Wed, Mar 08, 2023 at 05:37:11PM -0500, Aaron Conole wrote:
> Open vSwitch generally tries to let the underlying operating system
> managed the low level details of hardware, for example DMA mapping,
> bus arbitration, etc. However, when using DPDK, the underlying
> operating system yields control of many of these details to userspace
> for management.
>
> In the case of some DPDK port drivers, configuring rte_flow or even
> allocating resources may require access to iopl/ioperm calls, which
> are guarded by the CAP_SYS_RAWIO privilege on linux systems. These
> calls are dangerous, and can allow a process to completely compromise
> a system. However, they are needed in the case of some userspace
> driver code which manages the hardware (for example, the mlx
> implementation of backend support for rte_flow).
>
> Here, we create an opt-in flag passed to the command line to allow
> this access. We need to do this before ever accessing the database,
> because we want to drop all privileges asap, and cannot wait for
> a connection to the database to be established and functional before
> dropping. There may be distribution specific ways to do capability
> management as well (using for example, systemd), but they are not
> as universal to the vswitchd as a flag.
>
> Reviewed-by: Simon Horman
> Signed-off-by: Aaron Conole
> ---
> v1->v2: update daemon-windows for daemon_become_new_user
>
> v2->v3: update daemon-windows for daemon_start
> change log messages to be clearer
> update the manpage to provide example of why
> one would want the flag
>
> NEWS | 4
> lib/daemon-unix.c | 29 +
> lib/daemon-windows.c | 6 --
> lib/daemon.c | 2 +-
> lib/daemon.h | 4 ++--
> ovsdb/ovsdb-client.c | 6 +++---
> ovsdb/ovsdb-server.c | 4 ++--
> tests/test-netflow.c | 2 +-
> tests/test-sflow.c | 2 +-
> tests/test-unixctl.c | 2 +-
> utilities/ovs-ofctl.c | 4 ++--
> utilities/ovs-testcontroller.c | 4 ++--
> vswitchd/ovs-vswitchd.8.in | 9 +
> vswitchd/ovs-vswitchd.c| 11 ++-
> 14 files changed, 63 insertions(+), 26 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 85b3496214..65f35dcdd5 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,10 @@ Post-v3.1.0
> in order to create OVSDB sockets with access mode of 0770.
> - QoS:
> * Added new configuration option 'jitter' for a linux-netem QoS type.
> + - DPDK:
> + * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
> + with the --hw-rawio-access command line option. This allows the
> + process extra privileges when mapping physical interconnect memory.
>
>
> v3.1.0 - 16 Feb 2023
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index 1a7ba427d7..dd839015ab 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -88,7 +88,8 @@ static bool switch_user = false;
> static uid_t uid;
> static gid_t gid;
> static char *user = NULL;
> -static void daemon_become_new_user__(bool access_datapath);
> +static void daemon_become_new_user__(bool access_datapath,
> + bool access_hardware_ports);
>
> static void check_already_running(void);
> static int lock_pidfile(FILE *, int command);
> @@ -443,13 +444,13 @@ monitor_daemon(pid_t daemon_pid)
> * daemonize_complete()) or that it failed to start up (by exiting with a
> * nonzero exit code). */
> void
> -daemonize_start(bool access_datapath)
> +daemonize_start(bool access_datapath, bool access_hardware_ports)
> {
> assert_single_threaded();
> daemonize_fd = -1;
>
> if (switch_user) {
> -daemon_become_new_user__(access_datapath);
> +daemon_become_new_user__(access_datapath, access_hardware_ports);
> switch_user = false;
> }
>
> @@ -807,7 +808,8 @@ daemon_become_new_user_unix(void)
> /* Linux specific implementation of daemon_become_new_user()
> * using libcap-ng. */
> static void
> -daemon_become_new_user_linux(bool access_datapath OVS_UNUSED)
> +daemon_become_new_user_linux(bool access_datapath OVS_UNUSED,
> + bool access_hardware_ports OVS_UNUSED)
> {
> #if defined __linux__ && HAVE_LIBCAPNG
> int ret;
> @@ -827,6 +829,16 @@ daemon_become_new_user_linux(bool access_datapath
> OVS_UNUSED)
> ret = capng_update(CAPNG_ADD, cap_sets, CAP_NET_ADMIN)
>|| capng_update(CAPNG_ADD, cap_sets, CAP_NET_RAW)
>|| capng_update(CAPNG_ADD, cap_sets,
> CAP_NET_BROADCAST);
> +#ifdef DPDK_NETDEV
> +if (access_hardware_ports && !ret) {
> +ret = capng_update(CAPNG_ADD, cap_sets, CAP_SYS_RAWIO);
> +VLOG_INFO("The Linux capability CAP_SYS_RAWIO enabled.");
Shouldn't it be ...
Re: [ovs-dev] [PATCH v3] dpdk: Allow retaining CAP_SYS_RAWIO privileges
0-day Robot writes:
> Bleep bloop. Greetings Aaron Conole, I am a robot and I have tried out your
> patch.
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting. See the details below.
>
>
> checkpatch:
> WARNING: Line is 98 characters long (recommended limit is 79)
> #113 FILE: lib/daemon-unix.c:839:
> VLOG_WARN("No driver requires Linux capability
> CAP_SYS_RAWIO, disabling it.");
>
> Lines checked: 415, Warnings: 1, Errors: 0
I chose to keep this string unsplit because it makes it easier when
'grep'ing. If you think I should respin to shorten the lines, I can do
that, too.
>
> Please check this out. If you feel there has been an error, please email
> acon...@redhat.com
>
> Thanks,
> 0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] dpdk: Allow retaining CAP_SYS_RAWIO privileges
Bleep bloop. Greetings Aaron Conole, I am a robot and I have tried out your
patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 98 characters long (recommended limit is 79)
#113 FILE: lib/daemon-unix.c:839:
VLOG_WARN("No driver requires Linux capability
CAP_SYS_RAWIO, disabling it.");
Lines checked: 415, Warnings: 1, Errors: 0
Please check this out. If you feel there has been an error, please email
acon...@redhat.com
Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3] dpdk: Allow retaining CAP_SYS_RAWIO privileges
Open vSwitch generally tries to let the underlying operating system
managed the low level details of hardware, for example DMA mapping,
bus arbitration, etc. However, when using DPDK, the underlying
operating system yields control of many of these details to userspace
for management.
In the case of some DPDK port drivers, configuring rte_flow or even
allocating resources may require access to iopl/ioperm calls, which
are guarded by the CAP_SYS_RAWIO privilege on linux systems. These
calls are dangerous, and can allow a process to completely compromise
a system. However, they are needed in the case of some userspace
driver code which manages the hardware (for example, the mlx
implementation of backend support for rte_flow).
Here, we create an opt-in flag passed to the command line to allow
this access. We need to do this before ever accessing the database,
because we want to drop all privileges asap, and cannot wait for
a connection to the database to be established and functional before
dropping. There may be distribution specific ways to do capability
management as well (using for example, systemd), but they are not
as universal to the vswitchd as a flag.
Reviewed-by: Simon Horman
Signed-off-by: Aaron Conole
---
v1->v2: update daemon-windows for daemon_become_new_user
v2->v3: update daemon-windows for daemon_start
change log messages to be clearer
update the manpage to provide example of why
one would want the flag
NEWS | 4
lib/daemon-unix.c | 29 +
lib/daemon-windows.c | 6 --
lib/daemon.c | 2 +-
lib/daemon.h | 4 ++--
ovsdb/ovsdb-client.c | 6 +++---
ovsdb/ovsdb-server.c | 4 ++--
tests/test-netflow.c | 2 +-
tests/test-sflow.c | 2 +-
tests/test-unixctl.c | 2 +-
utilities/ovs-ofctl.c | 4 ++--
utilities/ovs-testcontroller.c | 4 ++--
vswitchd/ovs-vswitchd.8.in | 9 +
vswitchd/ovs-vswitchd.c| 11 ++-
14 files changed, 63 insertions(+), 26 deletions(-)
diff --git a/NEWS b/NEWS
index 85b3496214..65f35dcdd5 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,10 @@ Post-v3.1.0
in order to create OVSDB sockets with access mode of 0770.
- QoS:
* Added new configuration option 'jitter' for a linux-netem QoS type.
+ - DPDK:
+ * ovs-vswitchd will keep the CAP_SYS_RAWIO capability when started
+ with the --hw-rawio-access command line option. This allows the
+ process extra privileges when mapping physical interconnect memory.
v3.1.0 - 16 Feb 2023
diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
index 1a7ba427d7..dd839015ab 100644
--- a/lib/daemon-unix.c
+++ b/lib/daemon-unix.c
@@ -88,7 +88,8 @@ static bool switch_user = false;
static uid_t uid;
static gid_t gid;
static char *user = NULL;
-static void daemon_become_new_user__(bool access_datapath);
+static void daemon_become_new_user__(bool access_datapath,
+ bool access_hardware_ports);
static void check_already_running(void);
static int lock_pidfile(FILE *, int command);
@@ -443,13 +444,13 @@ monitor_daemon(pid_t daemon_pid)
* daemonize_complete()) or that it failed to start up (by exiting with a
* nonzero exit code). */
void
-daemonize_start(bool access_datapath)
+daemonize_start(bool access_datapath, bool access_hardware_ports)
{
assert_single_threaded();
daemonize_fd = -1;
if (switch_user) {
-daemon_become_new_user__(access_datapath);
+daemon_become_new_user__(access_datapath, access_hardware_ports);
switch_user = false;
}
@@ -807,7 +808,8 @@ daemon_become_new_user_unix(void)
/* Linux specific implementation of daemon_become_new_user()
* using libcap-ng. */
static void
-daemon_become_new_user_linux(bool access_datapath OVS_UNUSED)
+daemon_become_new_user_linux(bool access_datapath OVS_UNUSED,
+ bool access_hardware_ports OVS_UNUSED)
{
#if defined __linux__ && HAVE_LIBCAPNG
int ret;
@@ -827,6 +829,16 @@ daemon_become_new_user_linux(bool access_datapath
OVS_UNUSED)
ret = capng_update(CAPNG_ADD, cap_sets, CAP_NET_ADMIN)
|| capng_update(CAPNG_ADD, cap_sets, CAP_NET_RAW)
|| capng_update(CAPNG_ADD, cap_sets, CAP_NET_BROADCAST);
+#ifdef DPDK_NETDEV
+if (access_hardware_ports && !ret) {
+ret = capng_update(CAPNG_ADD, cap_sets, CAP_SYS_RAWIO);
+VLOG_INFO("The Linux capability CAP_SYS_RAWIO enabled.");
+}
+#else
+if (access_hardware_ports) {
+VLOG_WARN("No driver requires Linux capability
CAP_SYS_RAWIO, disabling it.");
+}
+#endif
}
} else {
ret = -1;
@@ -854,7 +866,7 @@ daemon_become_new_user_linux(bool
