Re: [PATCH v1] ksmbd: Fix user namespace mapping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 29/09/2022 13:37, Christian Brauner wrote:
On Thu, Sep 29, 2022 at 12:04:47PM +0200, Mickaël Salaün wrote:
A kernel daemon should not rely on the current thread, which is unknown
and might be malicious.  Before this security fix,
ksmbd_override_fsids() didn't correctly override FS UID/GID which means
that arbitrary user space threads could trick the kernel to impersonate
arbitrary users or groups for file system access checks, leading to
file system access bypass.

This was found while investigating truncate support for Landlock:
https://lore.kernel.org/r/CAKYAXd8fpMJ7guizOjHgxEyyjoUwPsx3jLOPZP=wPYcbhkVXqA@xxxxxxxxxxxxxx

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: Hyunchul Lee <hyc.lee@xxxxxxxxx>
Cc: Namjae Jeon <linkinjeon@xxxxxxxxxx>
Cc: Steve French <smfrench@xxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx>
Link: https://lore.kernel.org/r/20220929100447.108468-1-mic@xxxxxxxxxxx
---

I think this is ok. The alternative would probably be to somehow use a
relevant userns when struct ksmbd_user is created when the session is
established. But these are deeper ksmbd design questions. The fix
proposed here itself seems good.

That would be better indeed. I guess ksmbd works whenever the netlink peer is not in a user namespace with mapped UID/GID, but it should result in obvious access bugs otherwise (which is already the case anyway). It seems that the netlink peer must be trusted because it is the source of truth for account/user mapping anyway. This change fixes the more critical side of the issue and it should fit well for backports.



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux