From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753008AbbBSLME (ORCPT ); Thu, 19 Feb 2015 06:12:04 -0500 Received: from mail-wg0-f48.google.com ([74.125.82.48]:42667 "EHLO mail-wg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752075AbbBSLMC (ORCPT ); Thu, 19 Feb 2015 06:12:02 -0500 Message-ID: <54E5C4FE.5080604@linaro.org> Date: Thu, 19 Feb 2015 11:11:58 +0000 From: Srinivas Kandagatla User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Mark Brown CC: Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/2] regmap: Add range check in _regmap_raw_write() References: <1424335193-7431-1-git-send-email-srinivas.kandagatla@linaro.org> <1424335255-7517-1-git-send-email-srinivas.kandagatla@linaro.org> <20150219103156.GD3198@finisterre.sirena.org.uk> In-Reply-To: <20150219103156.GD3198@finisterre.sirena.org.uk> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 19/02/15 10:31, Mark Brown wrote: > On Thu, Feb 19, 2015 at 08:40:55AM +0000, Srinivas Kandagatla wrote: >> regmap_bulk_write() ends up using the path that invokes _regmap_raw_write(), >> however _regmap_raw_write() never checks if the registers that are accessed >> are actually within the accessible range. This results in kernel crashes when >> trying to access registers beyond max_registers. >> >> This patch just adds check before accessing the register range. > >> /* Check for unwritable registers before we start */ >> - if (map->writeable_reg) >> - for (i = 0; i < val_len / map->format.val_bytes; i++) >> - if (!map->writeable_reg(map->dev, >> - reg + (i * map->reg_stride))) >> - return -EINVAL; >> + for (i = 0; i < count; i++) >> + if (!regmap_writeable(map, reg + (i * map->reg_stride))) >> + return -EINVAL; > > Your changelog doesn't correspond to what the code is actually doing > here... what you're actually doing here is replacing an open coding of > regmap_writeable() with calls to the function. > > The same papering over the cracks concerns do apply here as well, it's > not immediately obvious that this is a good fix for the issue you > describe. Only reason for me to send this patch was that fact that _regmap_raw_write() also suffers from same issue as _regmap_raw_read(), which is "access beyond max_register". Should I drop this patch? Or Adding similar check of max_register before the writing makes sense? --srini