Linus (aka Greg),
It was reported that trace_printk() was not reporting properly
values that came after a dereference pointer.
trace_printk() utilizes vbin_printf() and bstr_printf() to keep the
overhead of tracing down. vbin_printf() does not do any conversions
and just stors the string format and the raw arguments into the
buffer. bstr_printf() is used to read the buffer and does the conversions
to complete the printf() output.
This can be troublesome with dereferenced pointers because the reference
may be different from the time vbin_printf() is called to the time
bstr_printf() is called. To fix this, a prior commit changed vbin_printf()
to convert dereferenced pointers into strings and load the converted
string into the buffer. But the change to bstr_printf() had an off-by-one
error and didn't account for the nul character at the end of the string
and this corrupted the rest of the values in the format that came after
a dereferenced pointer.
Please pull the latest trace-v4.19-rc5 tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.19-rc5
Tag SHA1: b5fc80d980ae316323e88c165084deef39afd168
Head SHA1: 62165600ae73ebd76e2d9b992b36360408d570d8
Steven Rostedt (VMware) (1):
vsprintf: Fix off-by-one bug in bstr_printf() processing dereferenced pointers
----
lib/vsprintf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---------------------------
commit 62165600ae73ebd76e2d9b992b36360408d570d8
Author: Steven Rostedt (VMware) <rostedt(a)goodmis.org>
Date: Fri Oct 5 10:08:03 2018 -0400
vsprintf: Fix off-by-one bug in bstr_printf() processing dereferenced pointers
The functions vbin_printf() and bstr_printf() are used by trace_printk() to
try to keep the overhead down during printing. trace_printk() uses
vbin_printf() at the time of execution, as it only scans the fmt string to
record the printf values into the buffer, and then uses vbin_printf() to do
the conversions to print the string based on the format and the saved
values in the buffer.
This is an issue for dereferenced pointers, as before commit 841a915d20c7b,
the processing of the pointer could happen some time after the pointer value
was recorded (reading the trace buffer). This means the processing of the
value at a later time could show different results, or even crash the
system, if the pointer no longer existed.
Commit 841a915d20c7b addressed this by processing dereferenced pointers at
the time of execution and save the result in the ring buffer as a string.
The bstr_printf() would then treat these pointers as normal strings, and
print the value. But there was an off-by-one bug here, where after
processing the argument, it move the pointer only "strlen(arg)" which made
the arg pointer not point to the next argument in the ring buffer, but
instead point to the nul character of the last argument. This causes any
values after a dereferenced pointer to be corrupted.
Cc: stable(a)vger.kernel.org
Fixes: 841a915d20c7b ("vsprintf: Do not have bprintf dereference pointers")
Reported-by: Nikolay Borisov <nborisov(a)suse.com>
Tested-by: Nikolay Borisov <nborisov(a)suse.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt(a)goodmis.org>
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index d5b3a3f95c01..812e59e13fe6 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2794,7 +2794,7 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
copy = end - str;
memcpy(str, args, copy);
str += len;
- args += len;
+ args += len + 1;
}
}
if (process)
Currently we return NOTIFY_DONE for any event which we don't think is
ours. However, many laptops will send more then just an ATIF event and
will also send an ACPI_VIDEO_NOTIFY_PROBE event as well. Since we don't
check for this, we return NOTIFY_DONE which causes a keypress for the
ACPI event to be propogated to userspace. This is the equivalent of
someone pressing the display key on a laptop every time there's a
hotplug event.
So, check for ACPI_VIDEO_NOTIFY_PROBE events and suppress keypresses
from them.
Signed-off-by: Lyude Paul <lyude(a)redhat.com>
Cc: stable(a)vger.kernel.org
---
drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index 353993218f21..f008804f0b97 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -358,7 +358,9 @@ static int amdgpu_atif_get_sbios_requests(struct amdgpu_atif *atif,
*
* Checks the acpi event and if it matches an atif event,
* handles it.
- * Returns NOTIFY code
+ *
+ * Returns:
+ * NOTIFY_BAD or NOTIFY_DONE, depending on the event.
*/
static int amdgpu_atif_handler(struct amdgpu_device *adev,
struct acpi_bus_event *event)
@@ -372,11 +374,16 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev,
if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0)
return NOTIFY_DONE;
+ /* Is this actually our event? */
if (!atif ||
!atif->notification_cfg.enabled ||
- event->type != atif->notification_cfg.command_code)
- /* Not our event */
- return NOTIFY_DONE;
+ event->type != atif->notification_cfg.command_code) {
+ /* These events will generate keypresses otherwise */
+ if (event->type == ACPI_VIDEO_NOTIFY_PROBE)
+ return NOTIFY_BAD;
+ else
+ return NOTIFY_DONE;
+ }
if (atif->functions.sbios_requests) {
struct atif_sbios_requests req;
@@ -385,7 +392,7 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev,
count = amdgpu_atif_get_sbios_requests(atif, &req);
if (count <= 0)
- return NOTIFY_DONE;
+ return NOTIFY_BAD;
DRM_DEBUG_DRIVER("ATIF: %d pending SBIOS requests\n", count);
--
2.17.1
xen_qlock_wait() isn't safe for nested calls due to interrupts. A call
of xen_qlock_kick() might be ignored in case a deeper nesting level
was active right before the call of xen_poll_irq():
CPU 1: CPU 2:
spin_lock(lock1)
spin_lock(lock1)
-> xen_qlock_wait()
-> xen_clear_irq_pending()
Interrupt happens
spin_unlock(lock1)
-> xen_qlock_kick(CPU 2)
spin_lock_irqsave(lock2)
spin_lock_irqsave(lock2)
-> xen_qlock_wait()
-> xen_clear_irq_pending()
clears kick for lock1
-> xen_poll_irq()
spin_unlock_irq_restore(lock2)
-> xen_qlock_kick(CPU 2)
wakes up
spin_unlock_irq_restore(lock2)
IRET
resumes in xen_qlock_wait()
-> xen_poll_irq()
never wakes up
The solution is to disable interrupts in xen_qlock_wait() and not to
poll for the irq in case xen_qlock_wait() is called in nmi context.
Cc: stable(a)vger.kernel.org
Cc: Waiman.Long(a)hp.com
Cc: peterz(a)infradead.org
Signed-off-by: Juergen Gross <jgross(a)suse.com>
---
arch/x86/xen/spinlock.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/arch/x86/xen/spinlock.c b/arch/x86/xen/spinlock.c
index cd210a4ba7b1..e8d880e98057 100644
--- a/arch/x86/xen/spinlock.c
+++ b/arch/x86/xen/spinlock.c
@@ -39,29 +39,25 @@ static void xen_qlock_kick(int cpu)
*/
static void xen_qlock_wait(u8 *byte, u8 val)
{
+ unsigned long flags;
int irq = __this_cpu_read(lock_kicker_irq);
/* If kicker interrupts not initialized yet, just spin */
- if (irq == -1)
+ if (irq == -1 || in_nmi())
return;
- /* If irq pending already clear it and return. */
+ /* Guard against reentry. */
+ local_irq_save(flags);
+
+ /* If irq pending already clear it. */
if (xen_test_irq_pending(irq)) {
xen_clear_irq_pending(irq);
- return;
+ } else if (READ_ONCE(*byte) == val) {
+ /* Block until irq becomes pending (or a spurious wakeup) */
+ xen_poll_irq(irq);
}
- if (READ_ONCE(*byte) != val)
- return;
-
- /*
- * If an interrupt happens here, it will leave the wakeup irq
- * pending, which will cause xen_poll_irq() to return
- * immediately.
- */
-
- /* Block until irq becomes pending (or perhaps a spurious wakeup) */
- xen_poll_irq(irq);
+ local_irq_restore(flags);
}
static irqreturn_t dummy_handler(int irq, void *dev_id)
--
2.16.4
Hi,
https://www.kernel.org is either down or very slow for me(based in The Netherlands, Europe).
I do understand this is not the right ML to report this issue, but the contact page doesn't load for
me and as a result I could find the right communication channel.
Cheers,
Pavlos