From patchwork Mon Nov 12 17:58:58 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bertrand Achard X-Patchwork-Id: 198435 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail-fa0-f56.google.com (mail-fa0-f56.google.com [209.85.161.56]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 155FE2C0087 for ; Tue, 13 Nov 2012 04:59:03 +1100 (EST) Received: by mail-fa0-f56.google.com with SMTP id p1sf1995759fad.11 for ; Mon, 12 Nov 2012 09:59:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=20120806; h=x-beenthere:received-spf:message-id:date:x-ovh-mailout:subject:from :to:cc:user-agent:mime-version:x-priority:importance:x-ovh-tracer-id :x-vr-spamstate:x-vr-spamscore:x-vr-spamcause:x-original-sender :x-original-authentication-results:reply-to:precedence:mailing-list :list-id:x-google-group-id:list-post:list-help:list-archive:sender :list-subscribe:list-unsubscribe:content-type; bh=G2rPcSJ47xATtD7uwDiBBU6lE/+JNhSJ1j+Aooi6Byg=; b=HzzjW5uSzb1z/lF1WBLIvyp45I8TdRFUB6zmZzMCQFTk4IwmGYqTRyz4UzJtbPdW7c KRfvM1S+gUuho90ZVds1Up8JJjdrxy9jpTAMGCwX4SXjrgvK6DHYqh6VZsxgC1oUgDcp NuOW+wJoZPxRpgK7753XFVpg1EQv3Og9m41zaqWZFLUc3prWpqBI3GqLbKvqqSMnS4uh 5lZgd7DrFt0vAwG7zg/ZaFN57gEU5+BOdn2daamK2bC+4q/aXNBNI8KzZmzKExSmaNL8 6dDA1m0NmB5I3r3Tp4cXg92QBoHersIA3pXxi05HQExsK6EvAcXAxpbJ0/XYmaSqjEg4 LgCw== Received: by 10.180.106.130 with SMTP id gu2mr1661730wib.0.1352743140518; Mon, 12 Nov 2012 09:59:00 -0800 (PST) X-BeenThere: rtc-linux@googlegroups.com Received: by 10.181.13.235 with SMTP id fb11ls930674wid.2.canary; Mon, 12 Nov 2012 09:58:58 -0800 (PST) Received: by 10.180.91.34 with SMTP id cb2mr2824662wib.3.1352743138923; Mon, 12 Nov 2012 09:58:58 -0800 (PST) Received: by 10.180.91.34 with SMTP id cb2mr2824661wib.3.1352743138909; Mon, 12 Nov 2012 09:58:58 -0800 (PST) Received: from mo6.mail-out.ovh.net (12.mo6.mail-out.ovh.net. [178.32.125.228]) by gmr-mx.google.com with ESMTP id g1si690327wie.0.2012.11.12.09.58.58; Mon, 12 Nov 2012 09:58:58 -0800 (PST) Received-SPF: neutral (google.com: 178.32.125.228 is neither permitted nor denied by best guess record for domain of ba@cykian.net) client-ip=178.32.125.228; Received: from mail382.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo6.mail-out.ovh.net (Postfix) with SMTP id 8DC7DFF867D for ; Mon, 12 Nov 2012 19:07:09 +0100 (CET) Received: from b0.ovh.net (HELO queueout) (213.186.33.50) by b0.ovh.net with SMTP; 12 Nov 2012 17:58:58 -0000 Received: from ns0.ovh.net (HELO ssl0.ovh.net) (213.186.33.20) by ns0.ovh.net with SMTP; 12 Nov 2012 17:58:58 -0000 Received: from 88.176.37.91 (SquirrelMail authenticated user ba@cykian.net) by ssl0.ovh.net with HTTP; Mon, 12 Nov 2012 18:58:58 +0100 Message-ID: <8a33d5e0f1f90b42ccd14bcdf89d8348.squirrel@ssl0.ovh.net> Date: Mon, 12 Nov 2012 18:58:58 +0100 X-Ovh-Mailout: 178.32.228.6 (mo6.mail-out.ovh.net) Subject: [rtc-linux] [PATCH] rtc: ds1307: long block operations bugfix From: "Bertrand Achard" To: rtc-linux@googlegroups.com Cc: "Alessandro Zummo" , "James Chapman" , "David Brownell" , matthias.fuchs@esd-electronics.com User-Agent: SquirrelMail/1.4.21 MIME-Version: 1.0 X-Priority: 3 (Normal) Importance: Normal X-Ovh-Tracer-Id: 4098838611694000049 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrfeehgedrtddtucetufdoteggodetrfcurfhrohhfihhlvgemucfqggfjnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfhrhhomhepfdeuvghrthhrrghnugcutegthhgrrhgufdcuoegsrgestgihkhhirghnrdhnvghtqeenucfjughrpefuhffvfgggtgfgrfgksehtkedttddtufdu X-Original-Sender: ba@cykian.net X-Original-Authentication-Results: gmr-mx.google.com; spf=neutral (google.com: 178.32.125.228 is neither permitted nor denied by best guess record for domain of ba@cykian.net) smtp.mail=ba@cykian.net Reply-To: rtc-linux@googlegroups.com Precedence: list Mailing-list: list rtc-linux@googlegroups.com; contact rtc-linux+owners@googlegroups.com List-ID: X-Google-Group-Id: 712029733259 List-Post: , List-Help: , List-Archive: Sender: rtc-linux@googlegroups.com List-Subscribe: , List-Unsubscribe: , Hello, The rtc-ds1307 driver does not properly handle block operations bigger than 32 bytes in either of the two modes supported (SMbus native, or emulated if not supported by the SMbus platform driver). It also does not properly handle userland-supplied input (block operation length) through sysfs and may suffer a type of buffer overrun. The driver has been modified with proper input validation, buffer sizes, and now splits block transfers bigger than 32 bytes into separate transfers. Explanation : Buffer size allocated is I2C_SMBUS_BLOCK_MAX which equals to 32 as per the SMbus spec. Reads and write may be up to 56 bytes (to the NVRAM). This patch allocated a 255 byte buffer, the maximum allowable (address is an u8). It's not only a buffer problem, SMbus only supports up to 32 bytes transfer at once, so it's needed to split bigger transfers. Patch successfully tested on 3.2.27; cleanly applies on 3.7-rc4. Signed-off-by: Bertrand Achard --- @@ -208,7 +209,7 @@ static s32 ds1307_read_block_data(const static s32 ds1307_write_block_data(const struct i2c_client *client, u8 command, u8 length, const u8 *values) { - u8 currvalues[I2C_SMBUS_BLOCK_MAX]; + u8 currvalues[255]; int tries = 0; dev_dbg(&client->dev, "ds1307_write_block_data (length=%d)\n", length); @@ -236,6 +237,57 @@ static s32 ds1307_write_block_data(const /*----------------------------------------------------------------------*/ +/* These RTC devices are not designed to be connected to a SMbus adapter. + SMbus limits block operations length to 32 bytes, whereas it's not + limited on I2C buses. As a result, accesses may exceed 32 bytes; + in that case, split them into smaller blocks */ + +static s32 ds1307_native_smbus_write_block_data(const struct i2c_client *client, + u8 command, u8 length, const u8 *values) +{ + if (length <= I2C_SMBUS_BLOCK_MAX) { + return i2c_smbus_write_i2c_block_data(client, + command, length, values); + } else { + u8 suboffset = 0; + while (suboffset < length) { + s32 retval = i2c_smbus_write_i2c_block_data(client, + command+suboffset, + min(I2C_SMBUS_BLOCK_MAX, length-suboffset), + values+suboffset); + if (retval < 0) + return retval; + + suboffset += I2C_SMBUS_BLOCK_MAX; + } + return length; + } +} + +static s32 ds1307_native_smbus_read_block_data(const struct i2c_client *client, + u8 command, u8 length, u8 *values) +{ + if (length <= I2C_SMBUS_BLOCK_MAX) { + return i2c_smbus_read_i2c_block_data(client, + command, length, values); + } else { + u8 suboffset = 0; + while (suboffset < length) { + s32 retval = i2c_smbus_read_i2c_block_data(client, + command+suboffset, + min(I2C_SMBUS_BLOCK_MAX, length-suboffset), + values+suboffset); + if (retval < 0) + return retval; + + suboffset += I2C_SMBUS_BLOCK_MAX; + } + return length; + } +} + +/*----------------------------------------------------------------------*/ + /* * The IRQ logic includes a "real" handler running in IRQ context just * long enough to schedule this workqueue entry. We need a task context @@ -641,8 +693,8 @@ static int __devinit ds1307_probe(struct buf = ds1307->regs; if (i2c_check_functionality(adapter, I2C_FUNC_SMBUS_I2C_BLOCK)) { - ds1307->read_block_data = i2c_smbus_read_i2c_block_data; - ds1307->write_block_data = i2c_smbus_write_i2c_block_data; + ds1307->read_block_data = ds1307_native_smbus_read_block_data; + ds1307->write_block_data = ds1307_native_smbus_write_block_data; } else { ds1307->read_block_data = ds1307_read_block_data; ds1307->write_block_data = ds1307_write_block_data; --- linux/drivers/rtc/rtc-ds1307.c.orig 2012-11-11 02:03:14.960707406 +0000 +++ linux/drivers/rtc/rtc-ds1307.c 2012-11-11 20:38:49.648717002 +0000 @@ -4,6 +4,7 @@ * Copyright (C) 2005 James Chapman (ds1337 core) * Copyright (C) 2006 David Brownell * Copyright (C) 2009 Matthias Fuchs (rx8025 support) + * Copyright (C) 2012 Bertrand Achard (nvram access fixes) * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -182,7 +183,7 @@ static s32 ds1307_read_block_data_once(c static s32 ds1307_read_block_data(const struct i2c_client *client, u8 command, u8 length, u8 *values) { - u8 oldvalues[I2C_SMBUS_BLOCK_MAX]; + u8 oldvalues[255]; s32 ret; int tries = 0;