linux-kernel - Re: Migration of kernel interfaces to seq

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m1bptp8pkk.fsf@fess.ebiederm.org>
Date:	Thu, 29 Jan 2009 19:01:31 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Paul Turner <pjt@...gle.com>
Cc:	Alexey Dobriyan <adobriyan@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: Migration of kernel interfaces to seq_files breaks pread() consumers

Paul Turner <pjt@...gle.com> writes:
>
> Thanks Eric,
>
> Moving the position into the seq_file structure is much cleaner.  Basic 
> tests seem to work ok.

Thanks.

> Few comments:
> - seq_open needs its fmode opened up to take advantage of this [patch 
> below]

Good point.
Acked-by: "Eric W. Biederman" <ebiederm@...ssion.com>
                                   

> - There's still the inherent problem of reads peturbing other reads (both 
> read and pread).  Our applications should be ok with this, however it does 
> deviate from the supposedly thread-safe pread semantics.

I don't see this not being thread safe.

If the data that is returned is stable.  We should not see anything
changing.  Otherwise it is all bets are off in any case.

Because seq_read happens under m->lock I don't see how there will
be anything thread unsafe.

The position that changes is just an internal implementation detail to
keep track of which data that we have cached.

>
> - Paul
>
> ---
>  fs/seq_file.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index cd63d69..aa3621f 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -48,8 +48,6 @@ int seq_open(struct file *file, const struct seq_operations
> *op)
>  	 */
>  	file->f_version = 0;
>  
> -	/* SEQ files support lseek, but not pread/pwrite */
> -	file->f_mode &= ~(FMODE_PREAD | FMODE_PWRITE);
>  	return 0;
>  }
>  EXPORT_SYMBOL(seq_open);
> -- 
> 1.5.4.5
--
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/