Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938739AbcKNVJ5 (ORCPT ); Mon, 14 Nov 2016 16:09:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56210 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753070AbcKNVJz (ORCPT ); Mon, 14 Nov 2016 16:09:55 -0500 Date: Mon, 14 Nov 2016 16:09:53 -0500 (EST) From: Mikulas Patocka X-X-Sender: mpatocka@file01.intranet.prod.int.rdu2.redhat.com To: Alexey Dobriyan cc: Milan Broz , Ondrej Kozina , Mike Snitzer , dm-devel@redhat.com, linux-kernel@vger.kernel.org Subject: Re: dm-crypt accepts '+' in the key In-Reply-To: <20161114003631.GA1304@avx2> Message-ID: References: <3ee83e71-78e0-7ba3-0fb4-5d906f66aa04@gmail.com> <20161114003631.GA1304@avx2> User-Agent: Alpine 2.02 (LRH 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 14 Nov 2016 21:09:55 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1574 Lines: 40 On Mon, 14 Nov 2016, Alexey Dobriyan wrote: > On Sun, Nov 13, 2016 at 03:45:27PM +0100, Milan Broz wrote: > > On 11/12/2016 09:20 PM, Mikulas Patocka wrote: > > > Hi > > > > > > dm-crypt uses the function kstrtou8 to decode the encryption key. kstrtou8 > > > calls kstrtoull and kstrtoull skips the first character if it is '+'. > > > > > > Consequently, it is possible to load keys with '+' in it. For example, > > > this is possible: > > > > > > dmsetup create cr --table "0 131072 crypt aes-cbc-essiv:sha256 +0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0+0 0 /dev/debian/tmptest 0" > > > > > > Should this be fixed in dm-crypt or in kstrtou8? A fix in kstrtou8 could > > > be more appropriate, but we don't know how many other kernel parts depend > > > on this "skip plus" behavior... > > > > I would way it should be checked in both places... > > For dmcrypt, it should validate input here and should > > not accept anything in key field in dm table that is not in hexa representation. > > > > (Is this regression since code switched from simple_strtoul to kstrtou8 > > or this bug was there always?) > > Well, before kernel would silently parse anything broken as "0". dm-crypt already validates that there are exactly two characters passed to kstrtou8 or simple_strtoul. > But since it is base-16, "0[xX]" will be accepted before every byte. Yes, the old dm-crypt code that used simple_strtoul accepted "0x" in a key (and parsed it as zero byte). It didn't accept "+" or "-". > dm-crypt should parse key by hand, frankly. Mikulas