[PATCH v2] net: qrtr: Expand control port access to root

Vishnu Santhosh posted 1 patch 1 month, 2 weeks ago
net/qrtr/af_qrtr.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH v2] net: qrtr: Expand control port access to root
Posted by Vishnu Santhosh 1 month, 2 weeks ago
When qrtr is loaded as module, QRTR NS runs from SELinux kmod_t
domain. On targets using upstream SELinux policies, this domain
does not receive CAP_NET_ADMIN, which prevents it from binding
control port even though QRTR NS is a trusted system component.

Granting kmod_t the CAP_NET_ADMIN capability in policy is possible,
but not desirable, as kmod_t is not expected to perform networking
operations and widening its capability set is discouraged.

To address this in a contained way within qrtr, extend the control
port permission check to allow binding when either:

  - the process has CAP_NET_ADMIN, or
  - the process belongs to GLOBAL_ROOT_GID (root-equivalent tasks)

This permits QRTR NS to successfully bind its control port in
kmod_t restricted environments without broadening SELinux capability
assignments.

Co-developed-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
---
Changes in v2:
- Replaced "qrtr-ns" with "QRTR NS" in the commit message to avoid
  confusion with the deprecated userspace qrtr-ns tool and the NS
  implementation inside the QRTR driver.
- Link to v1: https://lore.kernel.org/r/20260205-qrtr-control-port-access-permission-v1-1-e900039e92d5@oss.qualcomm.com
---
 net/qrtr/af_qrtr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index dab839f61ee93b876021d904ae6b8dca8ed43745..b0e252c16f156c05973988fbdf317a149ad9840d 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -8,6 +8,7 @@
 #include <linux/qrtr.h>
 #include <linux/termios.h>	/* For TIOCINQ/OUTQ */
 #include <linux/spinlock.h>
+#include <linux/uidgid.h>
 #include <linux/wait.h>
 
 #include <net/sock.h>
@@ -738,7 +739,8 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
 	if (!*port) {
 		rc = xa_alloc(&qrtr_ports, port, ipc, QRTR_EPH_PORT_RANGE,
 				GFP_KERNEL);
-	} else if (*port < QRTR_MIN_EPH_SOCKET && !capable(CAP_NET_ADMIN)) {
+	} else if (*port < QRTR_MIN_EPH_SOCKET && !(capable(CAP_NET_ADMIN) ||
+						   in_egroup_p(GLOBAL_ROOT_GID))) {
 		rc = -EACCES;
 	} else if (*port == QRTR_PORT_CTRL) {
 		rc = xa_insert(&qrtr_ports, 0, ipc, GFP_KERNEL);

---
base-commit: 0f2acd3148e0ef42bdacbd477f90e8533f96b2ac
change-id: 20260205-qrtr-control-port-access-permission-bfea19994a58

Best regards,
-- 
Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
Re: [PATCH v2] net: qrtr: Expand control port access to root
Posted by Bjorn Andersson 1 month, 2 weeks ago
On Mon, Feb 16, 2026 at 02:11:45PM +0530, Vishnu Santhosh wrote:
> When qrtr is loaded as module, QRTR NS runs from SELinux kmod_t
> domain. On targets using upstream SELinux policies, this domain
> does not receive CAP_NET_ADMIN, which prevents it from binding
> control port even though QRTR NS is a trusted system component.
> 

This part looks better now.

> Granting kmod_t the CAP_NET_ADMIN capability in policy is possible,
> but not desirable, as kmod_t is not expected to perform networking
> operations and widening its capability set is discouraged.
> 

Where is this discouraged? Who discouraged you do to this? Wasn't this
exactly what Stephen suggested you do in his feedback on v1?

Regards,
Bjorn

> To address this in a contained way within qrtr, extend the control
> port permission check to allow binding when either:
> 
>   - the process has CAP_NET_ADMIN, or
>   - the process belongs to GLOBAL_ROOT_GID (root-equivalent tasks)
> 
> This permits QRTR NS to successfully bind its control port in
> kmod_t restricted environments without broadening SELinux capability
> assignments.
> 
> Co-developed-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
> Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
> Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
> ---
> Changes in v2:
> - Replaced "qrtr-ns" with "QRTR NS" in the commit message to avoid
>   confusion with the deprecated userspace qrtr-ns tool and the NS
>   implementation inside the QRTR driver.
> - Link to v1: https://lore.kernel.org/r/20260205-qrtr-control-port-access-permission-v1-1-e900039e92d5@oss.qualcomm.com
> ---
>  net/qrtr/af_qrtr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
> index dab839f61ee93b876021d904ae6b8dca8ed43745..b0e252c16f156c05973988fbdf317a149ad9840d 100644
> --- a/net/qrtr/af_qrtr.c
> +++ b/net/qrtr/af_qrtr.c
> @@ -8,6 +8,7 @@
>  #include <linux/qrtr.h>
>  #include <linux/termios.h>	/* For TIOCINQ/OUTQ */
>  #include <linux/spinlock.h>
> +#include <linux/uidgid.h>
>  #include <linux/wait.h>
>  
>  #include <net/sock.h>
> @@ -738,7 +739,8 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
>  	if (!*port) {
>  		rc = xa_alloc(&qrtr_ports, port, ipc, QRTR_EPH_PORT_RANGE,
>  				GFP_KERNEL);
> -	} else if (*port < QRTR_MIN_EPH_SOCKET && !capable(CAP_NET_ADMIN)) {
> +	} else if (*port < QRTR_MIN_EPH_SOCKET && !(capable(CAP_NET_ADMIN) ||
> +						   in_egroup_p(GLOBAL_ROOT_GID))) {
>  		rc = -EACCES;
>  	} else if (*port == QRTR_PORT_CTRL) {
>  		rc = xa_insert(&qrtr_ports, 0, ipc, GFP_KERNEL);
> 
> ---
> base-commit: 0f2acd3148e0ef42bdacbd477f90e8533f96b2ac
> change-id: 20260205-qrtr-control-port-access-permission-bfea19994a58
> 
> Best regards,
> -- 
> Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
>
Re: [PATCH v2] net: qrtr: Expand control port access to root
Posted by Manivannan Sadhasivam 1 month, 2 weeks ago
On Mon, Feb 16, 2026 at 09:51:21AM -0600, Bjorn Andersson wrote:
> On Mon, Feb 16, 2026 at 02:11:45PM +0530, Vishnu Santhosh wrote:
> > When qrtr is loaded as module, QRTR NS runs from SELinux kmod_t
> > domain. On targets using upstream SELinux policies, this domain
> > does not receive CAP_NET_ADMIN, which prevents it from binding
> > control port even though QRTR NS is a trusted system component.
> > 
> 
> This part looks better now.
> 
> > Granting kmod_t the CAP_NET_ADMIN capability in policy is possible,
> > but not desirable, as kmod_t is not expected to perform networking
> > operations and widening its capability set is discouraged.
> > 
> 
> Where is this discouraged? Who discouraged you do to this? Wasn't this
> exactly what Stephen suggested you do in his feedback on v1?
> 

Yes. There is no point in respinning without addressing all the comments.

- Mani

> Regards,
> Bjorn
> 
> > To address this in a contained way within qrtr, extend the control
> > port permission check to allow binding when either:
> > 
> >   - the process has CAP_NET_ADMIN, or
> >   - the process belongs to GLOBAL_ROOT_GID (root-equivalent tasks)
> > 
> > This permits QRTR NS to successfully bind its control port in
> > kmod_t restricted environments without broadening SELinux capability
> > assignments.
> > 
> > Co-developed-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
> > Signed-off-by: Deepak Kumar Singh <deepak.singh@oss.qualcomm.com>
> > Signed-off-by: Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
> > ---
> > Changes in v2:
> > - Replaced "qrtr-ns" with "QRTR NS" in the commit message to avoid
> >   confusion with the deprecated userspace qrtr-ns tool and the NS
> >   implementation inside the QRTR driver.
> > - Link to v1: https://lore.kernel.org/r/20260205-qrtr-control-port-access-permission-v1-1-e900039e92d5@oss.qualcomm.com
> > ---
> >  net/qrtr/af_qrtr.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
> > index dab839f61ee93b876021d904ae6b8dca8ed43745..b0e252c16f156c05973988fbdf317a149ad9840d 100644
> > --- a/net/qrtr/af_qrtr.c
> > +++ b/net/qrtr/af_qrtr.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/qrtr.h>
> >  #include <linux/termios.h>	/* For TIOCINQ/OUTQ */
> >  #include <linux/spinlock.h>
> > +#include <linux/uidgid.h>
> >  #include <linux/wait.h>
> >  
> >  #include <net/sock.h>
> > @@ -738,7 +739,8 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
> >  	if (!*port) {
> >  		rc = xa_alloc(&qrtr_ports, port, ipc, QRTR_EPH_PORT_RANGE,
> >  				GFP_KERNEL);
> > -	} else if (*port < QRTR_MIN_EPH_SOCKET && !capable(CAP_NET_ADMIN)) {
> > +	} else if (*port < QRTR_MIN_EPH_SOCKET && !(capable(CAP_NET_ADMIN) ||
> > +						   in_egroup_p(GLOBAL_ROOT_GID))) {
> >  		rc = -EACCES;
> >  	} else if (*port == QRTR_PORT_CTRL) {
> >  		rc = xa_insert(&qrtr_ports, 0, ipc, GFP_KERNEL);
> > 
> > ---
> > base-commit: 0f2acd3148e0ef42bdacbd477f90e8533f96b2ac
> > change-id: 20260205-qrtr-control-port-access-permission-bfea19994a58
> > 
> > Best regards,
> > -- 
> > Vishnu Santhosh <vishnu.santhosh@oss.qualcomm.com>
> > 

-- 
மணிவண்ணன் சதாசிவம்