>* my patch guarantees that syslog(3) will only return valid data, i.e.
>it will refuse to return data which was already overwritten.
There's no one difference in returning random data or writing nothing to
the buffer. If you really want to fix this minor issue at the expense of
allocing a tmp-kernel buffer see the below patch.
>* register_console(CON_PRINTBUFFER) could use the wrong msg_level for
>the first line.
I think you are wrong. The current code seems fine.
>* console_print() must not use spin_lock_irq(), it needs
>spin_lock_irqsave().
Also all other functions (except do_syslog) needs the irqsave version.
Thanks for pointing this out.
Here it is my updated patch where I address the two points you raised
above.
Against 2.3.13 (but applyes cleanly also against 2.2.11):
--- 2.3.13/kernel/printk.c Thu Aug 12 02:53:25 1999
+++ 2.3.13-printk/kernel/printk.c Thu Aug 12 21:30:05 1999
@@ -17,6 +17,7 @@
#include <linux/smp_lock.h>
#include <linux/console.h>
#include <linux/init.h>
+#include <linux/slab.h>
#include <asm/uaccess.h>
@@ -46,6 +47,7 @@
static unsigned long logged_chars = 0;
struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
static int preferred_console = -1;
+spinlock_t console_lock = SPIN_LOCK_UNLOCKED;
/*
* Setup a list of consoles. Called from init/main.c
@@ -117,12 +119,12 @@
*/
int do_syslog(int type, char * buf, int len)
{
- unsigned long i, j, count, flags;
+ unsigned long i, j, count;
int do_clear = 0;
char c;
int error = -EPERM;
+ char * tmp_buf;
- lock_kernel();
error = 0;
switch (type) {
case 0: /* Close log */
@@ -143,18 +145,19 @@
if (error)
goto out;
i = 0;
+ spin_lock_irq(&console_lock);
while (log_size && i < len) {
c = *((char *) log_buf+log_start);
log_start++;
log_size--;
log_start &= LOG_BUF_LEN-1;
- sti();
+ spin_unlock_irq(&console_lock);
__put_user(c,buf);
buf++;
i++;
- cli();
+ spin_lock_irq(&console_lock);
}
- sti();
+ spin_unlock_irq(&console_lock);
error = i;
break;
case 4: /* Read/clear last kernel messages */
@@ -170,43 +173,65 @@
error = verify_area(VERIFY_WRITE,buf,len);
if (error)
goto out;
- /*
- * The logged_chars, log_start, and log_size values may
- * change from an interrupt, so we disable interrupts.
- */
- __save_flags(flags);
- __cli();
+ realloc_buf:
count = len;
if (count > LOG_BUF_LEN)
count = LOG_BUF_LEN;
+ spin_lock_irq(&console_lock);
if (count > logged_chars)
count = logged_chars;
+ spin_unlock_irq(&console_lock);
+ tmp_buf = kmalloc(count, GFP_KERNEL);
+ if (!tmp_buf)
+ goto out;
+ /* The logged_chars, log_start, and log_size are serialized
+ by the console_lock (the console_lock can be acquired also
+ from irqs by printk). */
+ spin_lock_irq(&console_lock);
+ if (count > logged_chars)
+ {
+ spin_unlock_irq(&console_lock);
+ kfree(tmp_buf);
+ goto realloc_buf;
+ }
j = log_start + log_size - count;
- __restore_flags(flags);
for (i = 0; i < count; i++) {
c = *((char *) log_buf+(j++ & (LOG_BUF_LEN-1)));
- __put_user(c, buf++);
+ *(tmp_buf+i) = c;
}
if (do_clear)
+ /* the increment done in printk may undo our
+ not atomic assigment if we do it without the
+ console lock held. */
logged_chars = 0;
+ spin_unlock_irq(&console_lock);
+ copy_to_user(buf, tmp_buf, count);
error = i;
break;
case 5: /* Clear ring buffer */
+ spin_lock_irq(&console_lock);
logged_chars = 0;
+ spin_unlock_irq(&console_lock);
break;
case 6: /* Disable logging to console */
+ spin_lock_irq(&console_lock);
console_loglevel = minimum_console_loglevel;
+ spin_unlock_irq(&console_lock);
break;
case 7: /* Enable logging to console */
+ spin_lock_irq(&console_lock);
console_loglevel = default_console_loglevel;
+ spin_unlock_irq(&console_lock);
break;
case 8:
error = -EINVAL;
if (len < 1 || len > 8)
goto out;
+ spin_lock_irq(&console_lock);
if (len < minimum_console_loglevel)
len = minimum_console_loglevel;
console_loglevel = len;
+ spin_unlock_irq(&console_lock);
error = 0;
break;
default:
@@ -214,7 +239,6 @@
break;
}
out:
- unlock_kernel();
return error;
}
@@ -226,8 +250,6 @@
}
-spinlock_t console_lock;
-
asmlinkage int printk(const char *fmt, ...)
{
va_list args;
@@ -292,24 +314,33 @@
void console_print(const char *s)
{
- struct console *c = console_drivers;
+ struct console *c;
int len = strlen(s);
+ unsigned long flags;
+ spin_lock_irqsave(&console_lock, flags);
+ c = console_drivers;
while(c) {
if ((c->flags & CON_ENABLED) && c->write)
c->write(c, s, len);
c = c->next;
}
+ spin_unlock_irqrestore(&console_lock, flags);
}
void unblank_console(void)
{
- struct console *c = console_drivers;
+ struct console *c;
+ unsigned long flags;
+
+ spin_lock_irqsave(&console_lock, flags);
+ c = console_drivers;
while(c) {
if ((c->flags & CON_ENABLED) && c->unblank)
c->unblank();
c = c->next;
}
+ spin_unlock_irqrestore(&console_lock, flags);
}
/*
@@ -321,10 +352,11 @@
void register_console(struct console * console)
{
int i,j,len;
- int p = log_start;
+ int p;
char buf[16];
signed char msg_level = -1;
char *q;
+ unsigned long flags;
/*
* See if we want to use this console driver. If we
@@ -370,6 +402,7 @@
* Put this console in the list - keep the
* preferred driver at the head of the list.
*/
+ spin_lock_irqsave(&console_lock, flags);
if ((console->flags & CON_CONSDEV) || console_drivers == NULL) {
console->next = console_drivers;
console_drivers = console;
@@ -377,12 +410,15 @@
console->next = console_drivers->next;
console_drivers->next = console;
}
+ spin_unlock_irqrestore(&console_lock, flags);
+
if ((console->flags & CON_PRINTBUFFER) == 0) return;
/*
* Print out buffered log messages.
*/
- for (i=0,j=0; i < log_size; i++) {
+ spin_lock_irqsave(&console_lock, flags);
+ for (p=log_start,i=0,j=0; i < log_size; i++) {
buf[j++] = log_buf[p];
p++; p &= LOG_BUF_LEN-1;
if (buf[j-1] != '\n' && i < log_size - 1 && j < sizeof(buf)-1)
@@ -401,26 +437,32 @@
msg_level = -1;
j = 0;
}
+ spin_unlock_irqrestore(&console_lock, flags);
}
int unregister_console(struct console * console)
{
struct console *a,*b;
+ int ret = 0;
+ unsigned long flags;
+ spin_lock_irqsave(&console_lock, flags);
if (console_drivers == console) {
console_drivers=console->next;
- return (0);
+ goto out;
}
for (a=console_drivers->next, b=console_drivers ;
a; b=a, a=b->next) {
if (a == console) {
b->next = a->next;
- return 0;
+ goto out;
}
}
-
- return (1);
+ ret = 1;
+ out:
+ spin_unlock_irqrestore(&console_lock, flags);
+ return ret;
}
/*
Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/