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] [day] [month] [year] [list]

Date:	Thu, 29 Jan 2009 22:09:55 -0800
From:	Paul Turner <pjt@...gle.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.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

On Thu, Jan 29, 2009 at 7:01 PM, Eric W. Biederman
<ebiederm@...ssion.com> wrote:
> 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.
>

With just reads you had stability on the record level at least.
Concurrent reads/seeks on a file made it obvious where things weren't
thread safe.

Obviously userspace doing stupid things isn't a kernel problem but it
could become one in the future if a previously safe concurrent pread
interface is migrated to use seqfile.

> 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.

I'm only referring to unstable records since every time we seek we
trash the cache.

>
>>
>> - 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/