Date: Mon, 16 Mar 2015 20:43:34 -0500 From: "Matthew D. Fuller" <fullermd@over-yonder.net> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: freebsd-geom@freebsd.org Subject: Re: RFC: Pass TRIM through GELI Message-ID: <20150317014334.GH52331@over-yonder.net> In-Reply-To: <20150316101843.GE52331@over-yonder.net> References: <20150308000131.GP1742@over-yonder.net> <20150314151453.GJ24274@over-yonder.net> <501bca86.508d8e26@fabiankeil.de> <20150315145324.GA52331@over-yonder.net> <20150315182444.GB52331@over-yonder.net> <20150316010845.GA1515@garage.freebsd.pl> <20150316092126.GC52331@over-yonder.net> <20150316100030.GB1515@garage.freebsd.pl> <20150316101323.GD52331@over-yonder.net> <20150316101843.GE52331@over-yonder.net>
next in thread | previous in thread | raw e-mail | index | archive | help
> [...] because in g_eli_read_metadata(), it doesn't check the return > from eli_metadata_decode(), so it doesn't notice the EINVAL and > happily reports back success without ever having touched the md :( Fixing that is enough to prevent the panic at any rate. At least in the normal case; if it randomly gets good magic, weird stuff might still happen. But that lack of checking is a Real Bug by itself anyway, so merits a fix. Index: g_eli.c =================================================================== --- g_eli.c (revision 280158) +++ g_eli.c (working copy) @@ -633,7 +633,9 @@ g_topology_lock(); if (buf == NULL) goto end; - eli_metadata_decode(buf, md); + error = eli_metadata_decode(buf, md); + if (error != 0) + goto end; end: if (buf != NULL) g_free(buf); (yes, the goto is redundant as all heck there. I decided to put it in for symmetry with the other checking, as well as for a JIC, if e.g. some code later got inserted after the decode before the end: label) So for the moment, I've reinstated the conditional in geli-delete.5.patch (which includes the above, though it's worthy of commission by itself anyway). Technically it shouldn't be necessary with that, since it'll fail on the reading anyway, but geli: Cannot change configuration of onetime provider ada0s1.nop. is friendlier and more informative than geli: Cannot read metadata from ada0s1.nop (error=22). and it also provides a convenient point to leave a comment about why it doesn't [yet] work, until it does. -- Matthew Fuller (MF4839) | fullermd@over-yonder.net Systems/Network Administrator | http://www.over-yonder.net/~fullermd/ On the Internet, nobody can hear you scream.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150317014334.GH52331>