| Message ID | 20250116195526.2303758-1-ingyujang25@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selinux: Handle NULL return from selinux_inode in inode_security_rcu | expand |
On Thu, Jan 16, 2025 at 2:55 PM Ingyu Jang <ingyujang25@gmail.com> wrote: > > The 'selinux_inode' function may return NULL if 'inode->i_security' is > uninitialized or if 'inode->i_security' is NULL. > Previously, this scenario was not explicitly handled in > 'inode_security_rcu', which might lead to NULL Pointer dereference. > > This patch modifies 'inode_security_rcu' to call 'selinux_inode' and > check its return value. > If 'selinux_inode' returns NULL, '-EACCES' is returned to ensure > consistent error handling. > > Furthermore, analysis confirmed that all current usages of > 'inode_security_rcu' check the return value using 'IS_ERR', ensuring > compatibility with this change. > > Signed-off-by: Ingyu Jang <ingyujang25@gmail.com> Do you have an actual example where this is currently possible if SELinux is enabled? It shouldn't be possible, so that would be a kernel bug that should be fixed. > --- > security/selinux/hooks.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 171dd7fceac5..61c9191bafbe 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -310,11 +310,17 @@ static struct inode_security_struct *inode_security_novalidate(struct inode *ino > static struct inode_security_struct *inode_security_rcu(struct inode *inode, bool rcu) > { > int error; > + struct inode_security_struct *isec; > > error = __inode_security_revalidate(inode, NULL, !rcu); > if (error) > return ERR_PTR(error); > - return selinux_inode(inode); > + > + isec = selinux_inode(inode); > + if (!isec) > + return ERR_PTR(-EACCES); > + > + return isec; > } > > /* > -- > 2.34.1 >
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 171dd7fceac5..61c9191bafbe 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -310,11 +310,17 @@ static struct inode_security_struct *inode_security_novalidate(struct inode *ino static struct inode_security_struct *inode_security_rcu(struct inode *inode, bool rcu) { int error; + struct inode_security_struct *isec; error = __inode_security_revalidate(inode, NULL, !rcu); if (error) return ERR_PTR(error); - return selinux_inode(inode); + + isec = selinux_inode(inode); + if (!isec) + return ERR_PTR(-EACCES); + + return isec; } /*
The 'selinux_inode' function may return NULL if 'inode->i_security' is uninitialized or if 'inode->i_security' is NULL. Previously, this scenario was not explicitly handled in 'inode_security_rcu', which might lead to NULL Pointer dereference. This patch modifies 'inode_security_rcu' to call 'selinux_inode' and check its return value. If 'selinux_inode' returns NULL, '-EACCES' is returned to ensure consistent error handling. Furthermore, analysis confirmed that all current usages of 'inode_security_rcu' check the return value using 'IS_ERR', ensuring compatibility with this change. Signed-off-by: Ingyu Jang <ingyujang25@gmail.com> --- security/selinux/hooks.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)