Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 02 Apr 2020 09:41:34 +0200
Message-ID: <s5h5zeiwd01.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     linux-acpi@vger.kernel.org
Cc:     Zhang Rui <rui.zhang@intel.com>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Len Brown <lenb@kernel.org>, linux-kernel@vger.kernel.org
Subject: OOB access on ACPI processor thermal device via sysfs write
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3
 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

[ My post yesterday didn't seem go out properly, so I'm resending with
  a different MTA setup; sorry if you already received it ]

Hi,

after my recent patch [*], I started looking at the root cause more
closely, and found that it's a side-effect of the device
initialization order between acpi_processor_driver and cpufreq.
  [*] https://lore.kernel.org/linux-pm/20200330140859.12535-1-tiwai@suse.de/

In short:
  
- acpi_processor_driver_init() is called fairly early boot stage, and
  it registers a thermal device for each CPU. 

- A thermal device allocates a statistics table with the fixed size
  determined by get_max_state() ops call at its probe time.
  (The table entry is updated at each time a write to thermal state
  file happens.)

  For ACPI, processor_get_max_state() is called and it invokes
  acpi_processor_max_state() that calculates the max states depending
  on cpufreq, cpufreq_get_max_state().

  cpufreq_get_max_state() returns 0 at this stage because no cpufreq
  driver is available.  So the table size is set to 1.

- Later on, after cpufreq get initialized, the following sysfs write
  causes an OOB access and corrupts memory (eventually Oops):
    # echo 3 > /sys/class/thermal/cooling_device0/cur_state

  The reason is that now processor_get_max_state() returns 3 as
  cpufreq became available.  So the thermal driver believes as if it
  were a valid value, and updates the table accordingly although it
  overflows.


My patch above detects and avoids such an overflow, but it's no real
fix for the cause.  The proper fix needs either the shuffling of the
device creation order or some automatic resize of statistics table.

Do you guys have any suggestion how to solve it?

FWIW, I could reproduce the problem locally on my laptop with 5.6
kernel, and I believe this can be seen generically on most of
machines.


thanks,

Takashi