Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 30 Jan 2012 08:30:34 +0200
From:      Kostik Belousov <kostikbel@gmail.com>
To:        Bruce Evans <brde@optusnet.com.au>, Gleb Smirnoff <glebius@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r230583 - head/sys/kern
Message-ID:  <20120130063034.GU2726@deviant.kiev.zoral.com.ua>
In-Reply-To: <20120129223904.GA37483@zim.MIT.EDU>
References:  <201201261159.q0QBxma2086162@svn.freebsd.org> <20120126233110.C960@besplex.bde.org> <20120126153641.GA68112@FreeBSD.org> <20120127194612.H1547@besplex.bde.org> <20120127091244.GZ2726@deviant.kiev.zoral.com.ua> <20120127194221.GA25723@zim.MIT.EDU> <20120128123748.GD2726@deviant.kiev.zoral.com.ua> <20120129001225.GA32220@zim.MIT.EDU> <20120129062327.GK2726@deviant.kiev.zoral.com.ua> <20120129223904.GA37483@zim.MIT.EDU>

next in thread | previous in thread | raw e-mail | index | archive | help

--kf/S9EuyQTNnOWN+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Jan 29, 2012 at 05:39:04PM -0500, David Schultz wrote:
> On Sun, Jan 29, 2012, Kostik Belousov wrote:
> > On Sat, Jan 28, 2012 at 07:12:25PM -0500, David Schultz wrote:
> > > On Sat, Jan 28, 2012, Kostik Belousov wrote:
> > > > On Fri, Jan 27, 2012 at 02:42:21PM -0500, David Schultz wrote:
> > > > > On Fri, Jan 27, 2012, Kostik Belousov wrote:
> > > > > > On Fri, Jan 27, 2012 at 07:50:30PM +1100, Bruce Evans wrote:
> > > > > > > On Thu, 26 Jan 2012, Gleb Smirnoff wrote:
> > > > > > >=20
> > > > > > > >On Thu, Jan 26, 2012 at 11:53:57PM +1100, Bruce Evans wrote:
> > > > > > > >B> > @@ -1552,6 +1552,12 @@ aio_aqueue(struct thread *td, st=
ruct aio
> > > > > > > >B> > 		return (error);
> > > > > > > >B> > 	}
> > > > > > > >B> >
> > > > > > > >B> > +	/* XXX: aio_nbytes is later casted to signed types. */
> > > > > > > >B> > +	if ((int)aiocbe->uaiocb.aio_nbytes < 0) {
> > > > > > > >B>
> > > > > > > >B> This should avoid implementation-defined behaviour by che=
cking if
> > > > > > > >B>
> > > > > > > >B>  	(uncast)aiocbe->uaiocb.aio_nbytes > INT_MAX.
> > > > > > >=20
> > > > > > > >Is the attached patch okay?
> > > > > > >=20
> > > > > > > Yes.  It now matches the style used for read^Wsys_read() and =
friends.
> > > > > > > This used to have to fit the count in "int uio_resid".  uio_r=
esid now
> > > > > > > has type ssize_t, but for some reason the old INT_MAX limits =
remain.
> > > > > >=20
> > > > > > Well, I can revive the patch. I still think it is good to get r=
id of
> > > > > > the limit.
> > > > >=20
> > > > > The correct limit on the maximum size of a single read/write is
> > > > > SSIZE_MAX, but FreeBSD uses INT_MAX.  It's not safe to raise the
> > > > > limit yet, though, because of bugs in several filesystems.  For
> > > > > example, FFS copies uio_resid into a local variable of type int.
> > > > > I have some old patches that fix some of these issues for FFS and
> > > > > cd9660, but surely there are more places I didn't notice.
> > > > >=20
> > > > Absolutely agree.
> > > >=20
> > > > http://people.freebsd.org/~kib/misc/uio_resid.5.patch
> > >=20
> > > Nice.  You found a lot more than I've got in my tree, and you even
> > > fixed the return values.  There are at least a few more places to
> > > fix.  For instance, cd9660 and the NFS client pass uio_resid or
> > > iov_len to min(), which operates on ints.  (Incidentally, C11
> > > generics ought to make it possible to write type-generic min()
> > > and max() functions.)
> >=20
> > Thank you, http://people.freebsd.org/~kib/misc/uio_resid.6.patch
> > changed them to MIN().
>=20
> This looks good to me.  I tried to think of other places that you
> might have missed, and the only one that occurred to me is the
Might ? I think this is a blatant understate.

> pipe code.  sys_pipe.c has an `int orig_resid' and lots of bogus
> casts of iov_len and uio_resid to type u_int.  Some look harmless,
> although it appears that writing a multiple of 2^32 bytes might
> result in pipe_build_write_buffer() allocating a 0-length buffer.
>=20
> My only reservation is that raising the limit could unmask a
> kernel buffer overflow if we missed something, but I guess we have
> to cross that bridge some day anyway.
Yes, and it is an obvious reason why I am chicken to commit this for
so long time. One more place, if this is reasonable to count as 'one'
place, are the cdevsw methods. devfs passes uio down to the drivers.

http://people.freebsd.org/~kib/misc/uio_resid.7.patch
contains the sys_pipe.c changes.

--kf/S9EuyQTNnOWN+
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)

iEYEARECAAYFAk8mOQkACgkQC3+MBN1Mb4gyFACgwmM0edJpdiHSX0AkjYDFe/s4
MPkAn2qkORTyGrHonghDapF4hYmHfcKx
=YclY
-----END PGP SIGNATURE-----

--kf/S9EuyQTNnOWN+--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120130063034.GU2726>