Re: [PATCH] linux-2.5.39 - i8xx series chipsets patches (patch3)

Wim Van Sebroeck (wim@iguana.be)
Sun, 29 Sep 2002 17:13:05 +0200


Hi Alan,

> > this patch upgrades the i810-tco module to version 0.05. It fixes a possible timer_alive race,
> > adds expect close support, cleans up ioctls, removes some unused stuff and adds support for
> > the 82801DB and 82801E chipsets. patch2 (on pci_ids.h file) needs to be applied first.
>
> This code is clearly incorrect. Please work off the current 2.4 driver
> code because all the 2.5 watchdog code is obsolete and has security
> holes. Worse still - you just added another one.

Included the patch with the fix for the security hole. For the rest the code is based on the 2.4 driver
allready (I only missed your last patch :-( ).

Greetings,
Wim.

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.636 -> 1.637
# drivers/char/i810-tco.c 1.9 -> 1.10
# drivers/char/i810-tco.h 1.2 -> 1.3
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/09/29 wim@iguana.be 1.637
# [PATCH] 2.5.39 - i8xx series chipsets patches (patch 3)
#
# i810-tco: Upgrade to version 0.05 .
# Fix possible timer_alive race, add expect close support,
# clean up ioctls (WDIOC_GETSTATUS, WDIOC_GETBOOTSTATUS and
# WDIOC_SETOPTIONS), made i810tco_getdevice __init,
# removed boot_status, removed tco_timer_read,
# added support for 82801DB and 82801E chipset, general cleanup.
# --------------------------------------------
#
diff -Nru a/drivers/char/i810-tco.c b/drivers/char/i810-tco.c
--- a/drivers/char/i810-tco.c Sun Sep 29 15:43:43 2002
+++ b/drivers/char/i810-tco.c Sun Sep 29 15:43:43 2002
@@ -1,5 +1,5 @@
/*
- * i810-tco 0.04: TCO timer driver for i8xx chipsets
+ * i810-tco 0.05: TCO timer driver for i8xx chipsets
*
* (c) Copyright 2000 kernel concepts <nils@kernelconcepts.de>, All Rights Reserved.
* http://www.kernelconcepts.de
@@ -9,9 +9,9 @@
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
*
- * Neither kernel concepts nor Nils Faerber admit liability nor provide
- * warranty for any of this software. This material is provided
- * "AS-IS" and at no charge.
+ * Neither kernel concepts nor Nils Faerber admit liability nor provide
+ * warranty for any of this software. This material is provided
+ * "AS-IS" and at no charge.
*
* (c) Copyright 2000 kernel concepts <nils@kernelconcepts.de>
* developed for
@@ -24,18 +24,25 @@
* (See the intel documentation on http://developer.intel.com.)
* 82801AA & 82801AB chip : document number 290655-003, 290677-004,
* 82801BA & 82801BAM chip : document number 290687-002, 298242-005,
- * 82801CA & 82801CAM chip : document number 290716-001, 290718-001
+ * 82801CA & 82801CAM chip : document number 290716-001, 290718-001,
+ * 82801DB & 82801E chip : document number 290744-001, 273599-001
*
* 20000710 Nils Faerber
* Initial Version 0.01
* 20000728 Nils Faerber
- * 0.02 Fix for SMI_EN->TCO_EN bit, some cleanups
+ * 0.02 Fix for SMI_EN->TCO_EN bit, some cleanups
* 20011214 Matt Domsch <Matt_Domsch@dell.com>
* 0.03 Added nowayout module option to override CONFIG_WATCHDOG_NOWAYOUT
* Didn't add timeout option as i810_margin already exists.
* 20020224 Joel Becker, Wim Van Sebroeck
* 0.04 Support for 82801CA(M) chipset, timer margin needs to be > 3,
* add support for WDIOC_SETTIMEOUT and WDIOC_GETTIMEOUT.
+ * 20020412 Rob Radez <rob@osinvestor.com>, Wim Van Sebroeck
+ * 0.05 Fix possible timer_alive race, add expect close support,
+ * clean up ioctls (WDIOC_GETSTATUS, WDIOC_GETBOOTSTATUS and
+ * WDIOC_SETOPTIONS), made i810tco_getdevice __init,
+ * removed boot_status, removed tco_timer_read,
+ * added support for 82801DB and 82801E chipset, general cleanup.
*/

#include <linux/module.h>
@@ -55,9 +62,9 @@


/* Module and version information */
-#define TCO_VERSION "0.04"
+#define TCO_VERSION "0.05"
#define TCO_MODULE_NAME "i810 TCO timer"
-#define TCO_DRIVER_NAME TCO_MODULE_NAME " , " TCO_VERSION
+#define TCO_DRIVER_NAME TCO_MODULE_NAME ", v" TCO_VERSION

/* Default expire timeout */
#define TIMER_MARGIN 50 /* steps of 0.6sec, 3<n<64. Default is 30 seconds */
@@ -67,9 +74,8 @@

static int i810_margin = TIMER_MARGIN; /* steps of 0.6sec */

-MODULE_PARM (i810_margin, "i");
-MODULE_PARM_DESC(i810_margin, "Watchdog timeout in steps of 0.6sec, 3<n<64. Default = 50 (30 seconds)");
-
+MODULE_PARM(i810_margin, "i");
+MODULE_PARM_DESC(i810_margin, "i810-tco timeout in steps of 0.6sec, 3<n<64. Default = 50 (30 seconds)");

#ifdef CONFIG_WATCHDOG_NOWAYOUT
static int nowayout = 1;
@@ -85,8 +91,8 @@
* Timer active flag
*/

-static int timer_alive;
-static int boot_status;
+static unsigned long timer_alive;
+static char tco_expect_close;

/*
* Some TCO specific functions
@@ -170,32 +176,19 @@
}

/*
- * Read the current timer value
- */
-static unsigned char tco_timer_read (void)
-{
- return (inb (TCO1_RLD));
-}
-
-
-/*
* Allow only one person to hold it open
*/

static int i810tco_open (struct inode *inode, struct file *file)
{
- if (timer_alive)
+ if (test_and_set_bit(0, &timer_alive))
return -EBUSY;

- if (nowayout) {
- MOD_INC_USE_COUNT;
- }
/*
* Reload and activate timer
*/
tco_timer_reload ();
tco_timer_start ();
- timer_alive = 1;
return 0;
}

@@ -204,10 +197,14 @@
/*
* Shut off the timer.
*/
- if (nowayout) {
+ if (tco_expect_close == 42 && !nowayout) {
tco_timer_stop ();
- timer_alive = 0;
+ } else {
+ tco_timer_reload ();
+ printk(KERN_CRIT TCO_MODULE_NAME ": Unexpected close, not stopping watchdog!\n");
}
+ clear_bit(0, &timer_alive);
+ tco_expect_close = 0;
return 0;
}

@@ -218,10 +215,22 @@
if (ppos != &file->f_pos)
return -ESPIPE;

- /*
- * Refresh the timer.
- */
+ /* See if we got the magic character 'V' and reload the timer */
if (len) {
+ size_t i;
+
+ tco_expect_close = 0;
+
+ /* scan to see wether or not we got the magic character */
+ for (i = 0; i != len; i++) {
+ u8 c;
+ if(get_user(c, data+i))
+ return -EFAULT;
+ if (c == 'V')
+ tco_expect_close = 42;
+ }
+
+ /* someone wrote to us, we should reload the timer */
tco_timer_reload ();
return 1;
}
@@ -232,41 +241,53 @@
unsigned int cmd, unsigned long arg)
{
int new_margin, u_margin;
+ int options, retval = -EINVAL;

static struct watchdog_info ident = {
- WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
- 0,
- "i810 TCO timer"
+ options: WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
+ firmware_version: 0,
+ identity: "i810 TCO timer",
};
switch (cmd) {
- default:
- return -ENOTTY;
- case WDIOC_GETSUPPORT:
- if (copy_to_user
- ((struct watchdog_info *) arg, &ident, sizeof (ident)))
- return -EFAULT;
- return 0;
- case WDIOC_GETSTATUS:
- return put_user (tco_timer_read (),
- (unsigned int *) (int) arg);
- case WDIOC_GETBOOTSTATUS:
- return put_user (boot_status, (int *) arg);
- case WDIOC_KEEPALIVE:
- tco_timer_reload ();
- return 0;
- case WDIOC_SETTIMEOUT:
- if (get_user(u_margin, (int *) arg))
- return -EFAULT;
- new_margin = (u_margin * 10 + 5) / 6;
- if ((new_margin < 4) || (new_margin > 63))
- return -EINVAL;
- if (tco_timer_settimer((unsigned char)new_margin))
- return -EINVAL;
- i810_margin = new_margin;
- tco_timer_reload();
- /* Fall */
- case WDIOC_GETTIMEOUT:
- return put_user((int)(i810_margin * 6 / 10), (int *) arg);
+ default:
+ return -ENOTTY;
+ case WDIOC_GETSUPPORT:
+ if (copy_to_user
+ ((struct watchdog_info *) arg, &ident, sizeof (ident)))
+ return -EFAULT;
+ return 0;
+ case WDIOC_GETSTATUS:
+ case WDIOC_GETBOOTSTATUS:
+ return put_user (0, (int *) arg);
+ case WDIOC_SETOPTIONS:
+ if (get_user (options, (int *) arg))
+ return -EFAULT;
+ if (options & WDIOS_DISABLECARD) {
+ tco_timer_stop ();
+ retval = 0;
+ }
+ if (options & WDIOS_ENABLECARD) {
+ tco_timer_reload ();
+ tco_timer_start ();
+ retval = 0;
+ }
+ return retval;
+ case WDIOC_KEEPALIVE:
+ tco_timer_reload ();
+ return 0;
+ case WDIOC_SETTIMEOUT:
+ if (get_user (u_margin, (int *) arg))
+ return -EFAULT;
+ new_margin = (u_margin * 10 + 5) / 6;
+ if ((new_margin < 4) || (new_margin > 63))
+ return -EINVAL;
+ if (tco_timer_settimer ((unsigned char) new_margin))
+ return -EINVAL;
+ i810_margin = new_margin;
+ tco_timer_reload ();
+ /* Fall */
+ case WDIOC_GETTIMEOUT:
+ return put_user ((int)(i810_margin * 6 / 10), (int *) arg);
}
}

@@ -285,13 +306,15 @@
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801BA_10, PCI_ANY_ID, PCI_ANY_ID, },
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_0, PCI_ANY_ID, PCI_ANY_ID, },
{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801CA_12, PCI_ANY_ID, PCI_ANY_ID, },
+ { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801DB_0, PCI_ANY_ID, PCI_ANY_ID, },
+ { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82801E_0, PCI_ANY_ID, PCI_ANY_ID, },
{ 0, },
};
MODULE_DEVICE_TABLE (pci, i810tco_pci_tbl);

static struct pci_dev *i810tco_pci;

-static unsigned char i810tco_getdevice (void)
+static unsigned char __init i810tco_getdevice (void)
{
struct pci_dev *dev;
u8 val1, val2;
@@ -341,7 +364,6 @@
outb (val1, SMI_EN + 1);
/* Clear out the (probably old) status */
outb (0, TCO1_STS);
- boot_status = (int) inb (TCO2_STS);
outb (3, TCO2_STS);
return 1;
}
@@ -357,9 +379,9 @@
};

static struct miscdevice i810tco_miscdev = {
- WATCHDOG_MINOR,
- "watchdog",
- &i810tco_fops
+ minor: WATCHDOG_MINOR,
+ name: "watchdog",
+ fops: &i810tco_fops,
};

static int __init watchdog_init (void)
@@ -382,8 +404,8 @@
tco_timer_reload ();

printk (KERN_INFO TCO_DRIVER_NAME
- ": timer margin: %d sec (0x%04x)\n",
- (int) (i810_margin * 6 / 10), TCOBASE);
+ ": timer margin: %d sec (0x%04x) (nowayout=%d)\n",
+ (int) (i810_margin * 6 / 10), TCOBASE, nowayout);
return 0;
}

@@ -404,4 +426,6 @@
module_init(watchdog_init);
module_exit(watchdog_cleanup);

+MODULE_AUTHOR("Nils Faerber");
+MODULE_DESCRIPTION("TCO timer driver for i8xx chipsets");
MODULE_LICENSE("GPL");
diff -Nru a/drivers/char/i810-tco.h b/drivers/char/i810-tco.h
--- a/drivers/char/i810-tco.h Sun Sep 29 15:43:43 2002
+++ b/drivers/char/i810-tco.h Sun Sep 29 15:43:43 2002
@@ -1,5 +1,5 @@
/*
- * i810-tco 0.04: TCO timer driver for i8xx chipsets
+ * i810-tco 0.05: TCO timer driver for i8xx chipsets
*
* (c) Copyright 2000 kernel concepts <nils@kernelconcepts.de>, All Rights Reserved.
* http://www.kernelconcepts.de
@@ -9,9 +9,9 @@
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
*
- * Neither kernel concepts nor Nils Faerber admit liability nor provide
- * warranty for any of this software. This material is provided
- * "AS-IS" and at no charge.
+ * Neither kernel concepts nor Nils Faerber admit liability nor provide
+ * warranty for any of this software. This material is provided
+ * "AS-IS" and at no charge.
*
* (c) Copyright 2000 kernel concepts <nils@kernelconcepts.de>
* developed for
@@ -20,13 +20,8 @@
* TCO timer driver for i8xx chipsets
* based on softdog.c by Alan Cox <alan@redhat.com>
*
- * The TCO timer is implemented in the following I/O controller hubs:
- * (See the intel documentation on http://developer.intel.com.)
- * 82801AA & 82801AB chip : document number 290655-003, 290677-004,
- * 82801BA & 82801BAM chip : document number 290687-002, 298242-005,
- * 82801CA & 82801CAM chip : document number 290716-001, 290718-001
- *
- * For history see i810-tco.c
+ * For history and the complete list of supported I/O Controller Hub's
+ * see i810-tco.c
*/


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/