From owner-freebsd-geom@FreeBSD.ORG Tue Mar 17 01:43:42 2015 Return-Path: Delivered-To: freebsd-geom@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id DCDB87F4; Tue, 17 Mar 2015 01:43:42 +0000 (UTC) Received: from thyme.infocus-llc.com (thyme.infocus-llc.com [199.15.120.10]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id B0772895; Tue, 17 Mar 2015 01:43:42 +0000 (UTC) Received: from draco.over-yonder.net (c-75-65-60-66.hsd1.ms.comcast.net [75.65.60.66]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by thyme.infocus-llc.com (Postfix) with ESMTPSA id CC38137B403; Mon, 16 Mar 2015 20:43:34 -0500 (CDT) Received: by draco.over-yonder.net (Postfix, from userid 100) id 3l5cj21bS8zws; Mon, 16 Mar 2015 20:43:34 -0500 (CDT) Date: Mon, 16 Mar 2015 20:43:34 -0500 From: "Matthew D. Fuller" To: Pawel Jakub Dawidek Subject: Re: RFC: Pass TRIM through GELI Message-ID: <20150317014334.GH52331@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150316101843.GE52331@over-yonder.net> X-Editor: vi X-OS: FreeBSD User-Agent: Mutt/1.5.23-fullermd.4 (2014-03-12) X-Virus-Scanned: clamav-milter 0.98.6 at thyme.infocus-llc.com X-Virus-Status: Clean Cc: freebsd-geom@freebsd.org X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Mar 2015 01:43:43 -0000 > [...] 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.