lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Jun 2020 21:08:07 +0200 From: Dominique Martinet <asmadeus@...ewreck.org> To: Alexander Kapshuk <alexander.kapshuk@...il.com> Cc: ericvh@...il.com, lucho@...kov.net, davem@...emloft.net, kuba@...nel.org, v9fs-developer@...ts.sourceforge.net, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] net/9p: Fix sparse rcu warnings in client.c Alexander Kapshuk wrote on Thu, Jun 18, 2020: > Address sparse nonderef rcu warnings: > net/9p/client.c:790:17: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:790:17: expected struct spinlock [usertype] *lock > net/9p/client.c:790:17: got struct spinlock [noderef] <asn:4> * > net/9p/client.c:792:48: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:792:48: expected struct spinlock [usertype] *lock > net/9p/client.c:792:48: got struct spinlock [noderef] <asn:4> * > net/9p/client.c:872:17: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:872:17: expected struct spinlock [usertype] *lock > net/9p/client.c:872:17: got struct spinlock [noderef] <asn:4> * > net/9p/client.c:874:48: warning: incorrect type in argument 1 (different address spaces) > net/9p/client.c:874:48: expected struct spinlock [usertype] *lock > net/9p/client.c:874:48: got struct spinlock [noderef] <asn:4> * > > Signed-off-by: Alexander Kapshuk <alexander.kapshuk@...il.com> Thanks for this patch. >From what I can see, there are tons of other parts of the code doing the same noderef access pattern to access current->sighand->siglock and I don't see much doing that. A couple of users justify this by saying SLAB_TYPESAFE_BY_RCU ensures we'll always get a usable lock which won't be reinitialized however we access it... It's a bit dubious we'll get the same lock than unlock to me, so I agree to some change though. After a second look I think we should use something like the following: if (!lock_task_sighand(current, &flags)) warn & skip (or some error, we'd null deref if this happened currently); recalc_sigpending(); unlock_task_sighand(current, &flags); As you can see, the rcu_read_lock() isn't kept until the unlock so I'm not sure it will be enough to please sparse, but I've convinced myself current->sighand cannot change while we hold the lock and there just are too many such patterns in the kernel. Please let me know if I missed something or if there is an ongoing effort to change how this works; I'll wait for a v2. -- Dominique
Powered by blists - more mailing lists