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: Tue, 17 Aug 2021 20:34:02 +0200 From: Manfred Spraul <manfred@...orfullife.com> To: Rafael Aquini <aquini@...hat.com>, linux-kernel@...r.kernel.org Cc: Andrew Morton <akpm@...ux-foundation.org>, Davidlohr Bueso <dbueso@...e.de>, Waiman Long <llong@...hat.com> Subject: Re: [PATCH] ipc: replace costly bailout check in sysvipc_find_ipc() Hello Rafael, I still try to understand the code. It seems, it is more or less unchanged from 2009: | https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/patch/ipc/util.c?id=7ca7e564e049d8b350ec9d958ff25eaa24226352 | On 8/9/21 10:35 PM, Rafael Aquini wrote: > --- a/ipc/util.c > +++ b/ipc/util.c > @@ -788,21 +788,13 @@ struct pid_namespace *ipc_seq_pid_ns(struct seq_file *s) > static struct kern_ipc_perm *sysvipc_find_ipc(struct ipc_ids *ids, loff_t pos, > loff_t *new_pos) > { > - struct kern_ipc_perm *ipc; > - int total, id; > - > - total = 0; > - for (id = 0; id < pos && total < ids->in_use; id++) { > - ipc = idr_find(&ids->ipcs_idr, id); > - if (ipc != NULL) > - total++; > - } > + struct kern_ipc_perm *ipc = NULL; > + int max_idx = ipc_get_maxidx(ids); > > - ipc = NULL; > - if (total >= ids->in_use) > + if (max_idx == -1 || pos > max_idx) > goto out; > > - for (; pos < ipc_mni; pos++) { > + for (; pos <= max_idx; pos++) { > ipc = idr_find(&ids->ipcs_idr, pos); > if (ipc != NULL) { > rcu_read_lock(); The change looks as correct to me. But I'm still not sure that I really understand what the current code does: - first, loop over index values in the idr, starting from 0, count found entries. - if we found all entries before we are at index=pos: fail - then search up to ipc_nmi for the next entry with an index >=pos. - if something is found: use it. otherwise fail. It seems the code tries to avoid that we loop until ipc_mni after the last entry was found, and therefore we loop every time from 0. From what I see, the change looks to be correct: You now remove the first loop, and instead of searching until ipc_mni, the search ends at <= max_idx. I'll try to find some time to test it. But: What about using idr_get_next() instead of the idr_find()? We want to find the next used index, thus idr_get_next() should be even better than the for loop, ... -- Manfred
Powered by blists - more mailing lists