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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 14 Dec 2008 17:02:05 -0800 From: ebiederm@...ssion.com (Eric W. Biederman) To: Jiri Slaby <jirislaby@...il.com> Cc: oleg@...sign.ru, kenchen@...gle.com, Linux kernel mailing list <linux-kernel@...r.kernel.org> Subject: Re: broken do_each_pid_{thread,task} Jiri Slaby <jirislaby@...il.com> writes: > Hi, > > I'm getting > `if (type == PIDTYPE_PID)' is unreachable > warning from kernel/exit.c. The preprocessed code looks like: > do { > struct hlist_node *pos___; > if (pgrp != ((void *)0)) > for (LIST ITERATION) { > { > if (!((p->state & 4) != 0)) > continue; > retval = 1; > break; > } > if (PIDTYPE_PGID == PIDTYPE_PID) > break; > } > } while (0); > and it's obviously wrong. Actually the test: > if (PIDTYPE_PGID == PIDTYPE_PID) > break; Is technically ok. The compiler should optimize it out instead of warning. Although seeing the unexpected corner case it gets us into I think it would be good to reconsider this test. The break statement is also fine because the outer loop is only executed once so it simply functions as an enclosing block, and the break transfers control to where it should go. > After investigating this code usage all around, it's broken on many places > this or similar way. > > For do_each_pid_thread(), even this code snippet from fs/ioprio.c is broken > due to double do {} while expansion: > do_each_pid_thread(pgrp, PIDTYPE_PGID, p) { > ret = set_task_ioprio(p, ioprio); > if (ret) > break; > } while_each_pid_thread(pgrp, PIDTYPE_PGID, p); > > Any idea how to get rid of this issue? The double loop there is certainly an issue. I'm not quite convinced that the error handling is correct even with the break statement. But the break statement was written when the code was just a single loop, so the behavior is definitely not what we intended. However I also agree with Ken Chen's assessment that we need to loop over threads and not just the process group leaders in some cases such as setting the io-priority. With respect to error handling and IO priorities can we fix the error handling by doing what we do when we send a signal to a process group? That is note that there was an error, finish processing all of the other processes and then return the error? Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists