linux-kernel - Re: [PATCH v2 4/4] splice: fix updating sd->pos wrongly

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: <E1OHwZC-0002KS-EJ@pomaz-ex.szeredi.hu>
Date:	Fri, 28 May 2010 12:13:26 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	Changli Gao <xiaosuo@...il.com>
CC:	miklos@...redi.hu, axboe@...nel.dk, viro@...iv.linux.org.uk,
	mszeredi@...e.cz, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] splice: fix updating sd->pos wrongly

On Fri, 28 May 2010, Changli Gao wrote:
> On Fri, May 28, 2010 at 5:42 PM, Miklos Szeredi <miklos@...redi.hu> wrote:
> > On Wed, 26 May 2010, Changli Gao wrote:
> >> fix updating sd->pos wrongly.
> >>
> >> In error path, we don't need to updating sd->pos, if the file isn't seekable.
> >
> > This patch is nonsense.  Why should we handle sd->pos != 0 case
> > differently?
> >
> 
> If the in file isn't seekable, its splice_read won't update *ppos, so
> in the error path, we'd better not change it too. Otherwise, some
> assumption will go wrong.

That may be true, but the patch is still nonsense.

Look, your patch is updating/not updating sd->pos based on whether it
is zero or not.  It will prevent updating the position for sockets,
but it will also prevent updating the position for regular files if
the position is zero, which is really not what we want.

Since these patches modify the behavior of splice in subtle ways, it
would be good if you had some test program to verify that you aren't
breaking something.

Thanks,
Miklos
--
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/