| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Oct 2008 10:43:57 +0300
From: Jüri Reitel <juri.reitel@...wenthal.ee>
To: linux-kernel@...r.kernel.org
CC: David Brownell <david-b@...bell.net>,
Alessandro Zummo <alessandro.zummo@...ertech.it>,
rtc-linux@...glegroups.com
Subject: Re: [rtc-linux] Re: invalid default values in RTC chip
David Brownell wrote:
> On Thursday 16 October 2008, Alessandro Zummo wrote:
>
>> On Tue, 14 Oct 2008 12:00:21 +0300
>> Jüri Reitel <juri.reitel@...wenthal.ee> wrote:
>>
>>
>>>> My question relates to RTC driver linux/drivers/rtc/rtc-ds1307.c for
>>>> device m41t00. Driver probe function will fail if some of the chip's
>>>> registers contain invalid date time values i.e. if month register is 32
>>>> or minutes is 61.
>>>>
>
> This stuff?
>
> tmp = ds1307->regs[DS1307_REG_SECS];
> tmp = BCD2BIN(tmp & 0x7f);
> if (tmp > 60)
> goto exit_bad;
> tmp = BCD2BIN(ds1307->regs[DS1307_REG_MIN] & 0x7f);
> if (tmp > 60)
> goto exit_bad;
>
> tmp = BCD2BIN(ds1307->regs[DS1307_REG_MDAY] & 0x3f);
> if (tmp == 0 || tmp > 31)
> goto exit_bad;
>
> tmp = BCD2BIN(ds1307->regs[DS1307_REG_MONTH] & 0x1f);
> if (tmp == 0 || tmp > 12)
> goto exit_bad;
>
>
>
>>>> Is this correct behavior? Probe function's purpose is
>>>> to check if the device is as was assumed (this time RTC and it is). The
>>>> chip's values are incorrect but the chip works, even the m41t00 chip
>>>> manual states that after initial powerup (RTC battery power applied)
>>>> internal registers will contain random data.
>>>>
>>>> There are two solutions first is driver patch and another is i2c-dev and
>>>> i2cset tool to use from user space during bootup. Whitch one should be used?
>>>>
>> Given we transitioned to the new i2c model, which mandates
>> a platform device to be declared,
>>
>
> Actually "i2c_board_info", not a "platform_device" ...
>
>
>
>> there's no more the need
>> to check for the device in the probing function.
>>
>> this should be fixed in the driver.
>>
>
> Right. The code I snipped above dates from the old
> drivers/i2c/chips/ds1337.c code, as I recall, which
> was trying to defend against some random non-RTC chip
> sitting at the relevant address. As pretty much all
> legacy I2C drivers needed to do.
>
> With "new style" I2C drivers, no guessing is needed
> and such defenses are superfluos. It's fair to notice
> that no chip is actually present ... but if there's
> a chip there, it should be assumed to match the
> declaration Linux was given.
>
>
>
>> however, a driver may still return a failure when reading
>> the time if there are inappropriate values in the registers.
>>
>
> In this driver, ds1307_get_time() already has a
>
> return rtc_valid_tm(t);
>
> But in the just-merged alarm support, ds1307_read_alarm()
> doesn't do that. Actually that function should use a name
> like ds1337_read_alarm(), since the 1307 has no alarm!
>
>
>
>> the solution is to patch the driver for detection and then
>> use a recent hwclock on bootup to write the time if required.
>>
>
> Exactly. Feel free to submit a patch removing those
> probe() checks, and fixing the code returning the alarm
> setting too.
>
>
I included patch that just removes the RTC time registers check, when i
compiled the module and booted up with invalid values in RTC then system
automatically set system time and RTC time to 1.1.2000 (this is nice
because i dont have to use date and hwclock manually)
i checked the rtc_valid_tm function and saw that it also checks for the
year value among others but in ds1307_read_alarm (now this function is
renamed to ds1337_read_alarm) year is set to -1 with some other fields
so rtc_valid_tm function can not be used in ds1337_read_alarm function.
> - Dave
>
>
>
This patch removes rtc date time values check in probe function
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 162330b..2e87620 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -298,7 +298,7 @@ static int ds1307_set_time(struct device *dev,
struct rtc_time *t)
return 0;
}
-static int ds1307_read_alarm(struct device *dev, struct rtc_wkalrm *t)
+static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
{
struct i2c_client *client = to_i2c_client(dev);
struct ds1307 *ds1307 = i2c_get_clientdata(client);
@@ -353,7 +353,7 @@ static int ds1307_read_alarm(struct device *dev,
struct rtc_wkalrm *t)
return 0;
}
-static int ds1307_set_alarm(struct device *dev, struct rtc_wkalrm *t)
+static int ds1337_set_alarm(struct device *dev, struct rtc_wkalrm *t)
{
struct i2c_client *client = to_i2c_client(dev);
struct ds1307 *ds1307 = i2c_get_clientdata(client);
@@ -475,8 +475,8 @@ static int ds1307_ioctl(struct device *dev, unsigned
int cmd, unsigned long arg)
static const struct rtc_class_ops ds13xx_rtc_ops = {
.read_time = ds1307_get_time,
.set_time = ds1307_set_time,
- .read_alarm = ds1307_read_alarm,
- .set_alarm = ds1307_set_alarm,
+ .read_alarm = ds1337_read_alarm,
+ .set_alarm = ds1337_set_alarm,
.ioctl = ds1307_ioctl,
};
@@ -707,22 +707,6 @@ read_rtc:
break;
}
- tmp = ds1307->regs[DS1307_REG_SECS];
- tmp = bcd2bin(tmp & 0x7f);
- if (tmp > 60)
- goto exit_bad;
- tmp = bcd2bin(ds1307->regs[DS1307_REG_MIN] & 0x7f);
- if (tmp > 60)
- goto exit_bad;
-
- tmp = bcd2bin(ds1307->regs[DS1307_REG_MDAY] & 0x3f);
- if (tmp == 0 || tmp > 31)
- goto exit_bad;
-
- tmp = bcd2bin(ds1307->regs[DS1307_REG_MONTH] & 0x1f);
- if (tmp == 0 || tmp > 12)
- goto exit_bad;
-
tmp = ds1307->regs[DS1307_REG_HOUR];
switch (ds1307->type) {
case ds_1340:
@@ -779,13 +763,6 @@ read_rtc:
return 0;
-exit_bad:
- dev_dbg(&client->dev, "%s: %02x %02x %02x %02x %02x %02x %02x\n",
- "bogus register",
- ds1307->regs[0], ds1307->regs[1],
- ds1307->regs[2], ds1307->regs[3],
- ds1307->regs[4], ds1307->regs[5],
- ds1307->regs[6]);
exit_irq:
if (ds1307->rtc)
rtc_device_unregister(ds1307->rtc);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists