Date: Thu, 2 Jan 2014 15:13:08 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: Stanislav Sedov <stas@freebsd.org>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, Alfred Perlstein <bright@mu.org>, src-committers@freebsd.org Subject: Re: svn commit: r255219 - in head: contrib/tcpdump lib/libc lib/libc/capability lib/libc/include lib/libc/sys lib/libprocstat sbin/dhclient sbin/hastd sys/amd64/linux32 sys/bsm sys/cddl/compat/opensola... Message-ID: <20140102131308.GI59496@kib.kiev.ua> In-Reply-To: <20140102104904.GB1655@garage.freebsd.pl> References: <201309050009.r8509vsE061271@svn.freebsd.org> <67DFFD7B-01DE-4862-BED3-DD42EB92A8F4@freebsd.org> <20140102093322.GA1655@garage.freebsd.pl> <52C53F69.3040507@mu.org> <20140102104904.GB1655@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
--Tr2u91LvQ9NOAnv9
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Thu, Jan 02, 2014 at 11:49:04AM +0100, Pawel Jakub Dawidek wrote:
> On Thu, Jan 02, 2014 at 02:28:57AM -0800, Alfred Perlstein wrote:
> > On 1/2/14 1:33 AM, Pawel Jakub Dawidek wrote:
> > > On Wed, Jan 01, 2014 at 11:16:22PM -0800, Stanislav Sedov wrote:
> > >> On Sep 4, 2013, at 5:09 PM, Pawel Jakub Dawidek <pjd@FreeBSD.org> wr=
ote:
> > >>
> > >>> This commit also breaks compatibility with some existing Capsicum=
system calls,
> > >>> but I see no other way to do that. This should be fine as Capsicu=
m is still
> > >>> experimental and this change is not going to 9.x.
> > >> Hi!
> > >>
> > >> This change also increases the size of kinfo_file structure, which w=
on???t allow
> > >> programs not compiled against HEAD and working with kern.info.filede=
sc sysctl
> > >> to run properly on HEAD (e.g. 8.x, 9.x and 10.x jails won???t run pr=
operly on HEAD,
> > >> and it also broke valgrind). Is there absolutely no way to avoid ex=
tending the size
> > >> of this struct?
> > > Well, I made this change to have space for future cap_rights_t
> > > expension. I did that change for a major branch, so we don't have to =
do
> > > it in the middle of 10.x or to not block the work until 11.0.
> > >
> > > Note that the structure changed size not only because of _kf_cap_spar=
e[3]
> > > field, but also because cap_rights_t is not uint64_t anymore, it is n=
ow
> > > struct that contains two uint64_t (1424 - 1392 =3D 4 * 8).
> > >
> > > I'm afraid it is too late to change it for 10.0 at this point anyway.
> > > Not sure if you are aware this was merged to 10, because you write ab=
out
> > > 10.x jails not working properly on HEAD. 10.x jails will work properly
> > > on HEAD.
> > >
> > > BTW. I'd love if we stop using such structures for a running kernel.
> > > We should really move to using libnv to export data like that.
> >=20
> > Aren't there enough bits in int _kf_ispare[4]; /* Spac=
e=20
> > for more stuff. */
> > to make this work for the time being until you can provide an alternate=
=20
> > way to fetch the cap stuff from the kernel.
>=20
> I don't plan to provide alternative way to fetch the cap stuff. Well, I
> implemented libnv, which can be used to reimplement how we fetch all
> data like kinfo_file in a ABI friendly way, but I don't plan to modify
> this specific code myself.
I.e. you break something and decline to fix it, putting the burden on
somebody else.
>=20
> > Afaik you could just remove the "spare" and steal 2 or 4 entries from=
=20
> > _kf_ispare until it is sorted.
>=20
> Yes, this would work for current cap_rights_t structure, at least for
> i386 and amd64, but would only allow to expand the structure by one
> uint64_t in the future (which might or might not be enough). The
> cap_rights_t structure is designed to be expanded to 5 uint64_ts without
> breaking ABI. I don't want to stuck with current cap_rights_t that is
> designed to expand, but cannot be, because kinfo_file wasn't modified at
> the start of a major branch.
The ABI stability is not limited to the single branch. It must be
preserved across whole project lifetime.
>=20
> > Can you please make use of that and discuss merge to 10 with re@?
>=20
> I'm Bccing re@, but I'm pretty sure it is too late for such a change,
> especially that it breaks ABI with all 10-RCs. I'm also not changing my
> mind. I'd like to structure to stay as-is.
This is just awful breakage of _ABI_. We cannot leave it as is,
unless we also claim that project gave up on ABI stability at all.
>=20
> > It really sounds like breaking top/etc under jails is something that=20
> > should and can be avoided.
>=20
> I agree. Maybe it should be done every 10 major releases (I'm still fine
> with that rule), but we cannot just stuck with it forever.
>=20
> My suggestions would be:
> 1. Move to libnv.
> 2. Detect that the given binary was compiled against some older version
> of this structure and copy old structure to userland. Not sure if we
> can do that now or not, but I'd expect we can detect that.
The only way to fix this is either to stop passing caps in kinfo_file,
reverting it to the pre-commit state, or follow Alfred suggestion
by consuming spares.
My own opinion is that the kinfo change must be removed, and the bug
is so critical that another RC must be issued.
>=20
> > >>> #if defined(__amd64__) || defined(__i386__)
> > >>> -#define KINFO_FILE_SIZE 1392
> > >>> +#define KINFO_FILE_SIZE 1424
> > >>> #endif
> > >>> =20
> > >>> struct kinfo_file {
> > >>> @@ -389,6 +390,7 @@
> > >>> uint16_t kf_pad1; /* Round to 32 bit=
alignment. */
> > >>> int _kf_ispare0; /* Space for more =
stuff. */
> > >>> cap_rights_t kf_cap_rights; /* Capability righ=
ts. */
> > >>> + uint64_t _kf_cap_spare[3]; /* Space for future=
cap_rights_t. */
> > >>> int _kf_ispare[4]; /* Space for more =
stuff. */
> > >>> /* Truncated before copyout in sysctl */
> > >>> char kf_path[PATH_MAX]; /* Path to file, i=
f any. */
>=20
> --=20
> Pawel Jakub Dawidek http://www.wheelsystems.com
> FreeBSD committer http://www.FreeBSD.org
> Am I Evil? Yes, I Am! http://mobter.com
--Tr2u91LvQ9NOAnv9
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (FreeBSD)
iQIcBAEBAgAGBQJSxWXjAAoJEJDCuSvBvK1B9ugP/1c0ZNgQRdYy+y+exHeIBrt4
hwn/5fUYYSpSYD1F4pSfOdhOeBjSORDU2cGTS6izTtH6jFTPqKllYoWohNEQNoqV
juFGXeZSFSc7OB2wKtK9rV5H0maSFwX/6RAZtJyxxPIxyCYo3JRFnYQAWtEKMDwI
ccQzQrngFavBcJvpsifX1H8e4WvhEADkmzkRRs7wJidAIY4xGbp/Xc8Z7pPNFWLO
eSDMyqEEKmKTRlaDyxXfnrGU3AgCzKLj41Ag250oGAzRcrzq3SSxgNVsPSLxvmuE
YA2P0Px9UIrYfa8EQ7/7ZqNaliGqnKhNFFkZETmR2yBgzavnwfxr6CbdF2p8qiDZ
W1E0EnzFOmz+v2/jx3CT370tcQoz84/27oaXxc7R4I0kZten5SyKyrDp9AZSDVHJ
7xYED7Ew0yVSGEojY1W34w51uG0b2nUYupq0mOAs0rLsuLY71nYmz4YSrCdG08GY
wfS5rV0eiehv3oF6TRhVQYyUH17B16vJFGHCnf28im+ebyUOBLNgkEYRnPw3YpdH
haDstWPtOQv+5wIjk/8WwiufTXeUwSOxC5b9Itv5MYarO2ny8Sm5J21Eaa1KJ4V+
pJi9xMs+fSa71AlqLxae22VUmEV3ciuHYIyYdtY8SbNu3pQ99d0FlVHtVd7g4Fku
AgQ9v/cCrjw/+BnnYPa1
=pVrj
-----END PGP SIGNATURE-----
--Tr2u91LvQ9NOAnv9--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140102131308.GI59496>