All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters
Date: Tue, 5 Feb 2008 14:18:05 -0800	[thread overview]
Message-ID: <20080205221805.GC6950@suse.de> (raw)
In-Reply-To: <200802050230.10873.lenb@kernel.org>

On Tue, Feb 05, 2008 at 02:30:10AM -0500, Len Brown wrote:
> # cat /sys/firmware/acpi/interrupts/summary
> pm_timer     0
> glbl_lock    0
> power_btn    0
> sleep_btn    0
> rtc          0
> gpe00    0
> gpe01    0
> gpe02    0
> gpe03    0
> gpe04    0
> gpe05    0
> gpe06    0
> gpe07    0
> gpe08    0
> gpe09    2
> gpe0A    0
> gpe0B    0
> gpe0C    0
> gpe0D    0
> gpe0E    0
> gpe0F    0
> gpe10    0
> gpe11   60
> gpe12    0
> gpe13    0
> gpe14    0
> gpe15    0
> gpe16    0
> gpe17    0
> gpe18    0
> gpe19    1
> gpe1A    0
> gpe1B    0
> gpe1C    0
> gpe1D    0
> gpe1E    0
> gpe1F    0
> gpe_hi    0
> gpe_total   63
> acpi_irq    63

Eeek!  Why?  What's wrong with individual files here?  What's to ensure
that you aren't going to overflow your buffer?  There's a reason we
don't have the seq file interface for sysfs, to prevent this very kind
of thing...


> +	/*
> +	 * General Purpose Events
> +	 */
> +	for (i = 0; i < number_of_gpes; i++) {
> +		count += sprintf(buf + count, "gpe%02X %4d\n",
> +			i, acpi_gpe_counters[i]);
> +	}

Nope, no error checking, fun chance of blowing the memory buffer :(

Please change the interface to not do this.

Oh, and for every new sysfs file, you need a Documentation/ABI/
addition, please also add that.

thanks,

greg k-h

  reply	other threads:[~2008-02-05 22:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-05  7:30 [PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters Len Brown
2008-02-05 22:18 ` Greg KH [this message]
2008-02-05 23:12   ` Len Brown
2008-02-05 23:18     ` Greg KH
2008-02-06  1:58       ` Len Brown
2008-02-06  2:43         ` Greg KH
2008-02-06  6:33           ` Len Brown
2008-02-06  6:40             ` [PATCH] ACPI: create /sys/firmware/acpi/interrupts/ (v2) Len Brown
2008-02-07  0:16               ` Greg KH
2008-02-06  6:49             ` [PATCH for review] ACPI: Create /sys/firmware/acpi/interrupts/ counters Greg KH
2008-02-06  0:44     ` Bjorn Helgaas
2008-02-06  1:53       ` Len Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080205221805.GC6950@suse.de \
    --to=gregkh@suse.de \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.