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: Fri, 7 Apr 2017 22:37:01 +0200 From: Dmitry Vyukov <dvyukov@...gle.com> To: Akinobu Mita <akinobu.mita@...il.com> Cc: Andrew Morton <akpm@...ux-foundation.org>, LKML <linux-kernel@...r.kernel.org> Subject: Re: [PATCH -mm 3/5] fault-inject: make fail-nth read/write interface symmetric On Thu, Apr 6, 2017 at 4:55 PM, Akinobu Mita <akinobu.mita@...il.com> wrote: > The read interface for fail-nth looks a bit odd. Read from this file > returns "NYYYY..." or "YYYYY..." (this makes me surprise when cat this > file). Because there is no EOF condition. The first character indicates > current->fail_nth is zero or not, and then current->fail_nth is reset > to zero. > > Just returning task->fail_nth value is more natural to understand. > > Cc: Dmitry Vyukov <dvyukov@...gle.com> > Signed-off-by: Akinobu Mita <akinobu.mita@...il.com> > --- > Documentation/fault-injection/fault-injection.txt | 13 +++++++------ > fs/proc/base.c | 14 ++++++-------- > 2 files changed, 13 insertions(+), 14 deletions(-) > > diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt > index a321905..370ddcb 100644 > --- a/Documentation/fault-injection/fault-injection.txt > +++ b/Documentation/fault-injection/fault-injection.txt > @@ -139,9 +139,9 @@ o proc entries > - /proc/self/task/<current-tid>/fail-nth: > > Write to this file of integer N makes N-th call in the task fail. > - Read from this file returns a single char 'Y' or 'N' > - that says if the fault setup with a previous write to this file was > - injected or not, and disables the fault if it wasn't yet injected. > + Read from this file returns a integer value. A value of '0' indicates > + that the fault setup with a previous write to this file was injected. > + A positive integer N indicates that the fault wasn't yet injected. > Note that this file enables all types of faults (slab, futex, etc). > This setting takes precedence over all other generic debugfs settings > like probability, interval, times, etc. But per-capability settings > @@ -325,13 +325,14 @@ int main() > write(fail_nth, buf, strlen(buf)); > res = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds); > err = errno; > - read(fail_nth, buf, 1); > + pread(fail_nth, buf, sizeof(buf), 0); > if (res == 0) { > close(fds[0]); > close(fds[1]); > } > - printf("%d-th fault %c: res=%d/%d\n", i, buf[0], res, err); > - if (buf[0] != 'Y') > + printf("%d-th fault %c: res=%d/%d\n", i, atoi(buf) ? 'N' : 'Y', > + res, err); > + if (atoi(buf)) > break; > } > return 0; > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 42c52e2..9d14215 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -1383,7 +1383,8 @@ static ssize_t proc_fail_nth_read(struct file *file, char __user *buf, > size_t count, loff_t *ppos) > { > struct task_struct *task; > - int err; > + char numbuf[PROC_NUMBUF]; > + ssize_t len; > > task = get_proc_task(file_inode(file)); > if (!task) > @@ -1391,13 +1392,10 @@ static ssize_t proc_fail_nth_read(struct file *file, char __user *buf, > put_task_struct(task); > if (task != current) > return -EPERM; > - if (count < 1) > - return -EINVAL; > - err = put_user((char)(current->fail_nth ? 'N' : 'Y'), buf); > - if (err) > - return err; > - current->fail_nth = 0; > - return 1; > + len = snprintf(numbuf, sizeof(numbuf), "%u\n", task->fail_nth); If we allow setting this for non current task, then we need to prevent data races as the task uses task->fail_nth concurrently. Reads then should use READ_ONCE and writes in fault-inject.c should use WRITE_ONCE. > + len = simple_read_from_buffer(buf, count, ppos, numbuf, len); > + > + return len; > } > > static const struct file_operations proc_fail_nth_operations = { > -- > 2.7.4 >
Powered by blists - more mailing lists