* [PATCH v2 1/2] livepatch: use arch_klp_init_object_loaded() to finish arch-specific tasks
2016-07-21 5:23 [PATCH v2 0/2] Fix issue with alternatives/paravirt patches Jessica Yu
@ 2016-07-21 5:23 ` Jessica Yu
2016-08-02 8:25 ` Miroslav Benes
2016-07-21 5:24 ` [PATCH v2 2/2] livepatch/x86: apply alternatives and paravirt patches after relocations Jessica Yu
2016-07-26 20:59 ` [PATCH v2 0/2] Fix issue with alternatives/paravirt patches Josh Poimboeuf
2 siblings, 1 reply; 8+ messages in thread
From: Jessica Yu @ 2016-07-21 5:23 UTC (permalink / raw)
To: Josh Poimboeuf, Miroslav Benes, Petr Mladek, Jiri Kosina,
Chris J Arges, Eugene Shatokhin
Cc: live-patching, x86, linux-kernel, Jessica Yu
Introduce arch_klp_init_object_loaded() to complete any additional
arch-specific tasks during patching. Architecture code may override this
function.
Signed-off-by: Jessica Yu <jeyu@redhat.com>
---
include/linux/livepatch.h | 3 +++
kernel/livepatch/core.c | 12 ++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index a93a0b2..9072f04 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -116,6 +116,9 @@ int klp_unregister_patch(struct klp_patch *);
int klp_enable_patch(struct klp_patch *);
int klp_disable_patch(struct klp_patch *);
+void arch_klp_init_object_loaded(struct klp_patch *patch,
+ struct klp_object *obj);
+
/* Called from the module loader during module coming/going states */
int klp_module_coming(struct module *mod);
void klp_module_going(struct module *mod);
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 5c2bc10..164eff6 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -274,7 +274,6 @@ static int klp_write_object_relocations(struct module *pmod,
objname = klp_is_module(obj) ? obj->name : "vmlinux";
- module_disable_ro(pmod);
/* For each klp relocation section */
for (i = 1; i < pmod->klp_info->hdr.e_shnum; i++) {
sec = pmod->klp_info->sechdrs + i;
@@ -309,7 +308,6 @@ static int klp_write_object_relocations(struct module *pmod,
break;
}
- module_enable_ro(pmod);
return ret;
}
@@ -763,6 +761,12 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
func->old_sympos ? func->old_sympos : 1);
}
+/* Arches may override this to finish any remaining arch-specific tasks */
+void __weak arch_klp_init_object_loaded(struct klp_patch *patch,
+ struct klp_object *obj)
+{
+}
+
/* parts of the initialization that is done only when the object is loaded */
static int klp_init_object_loaded(struct klp_patch *patch,
struct klp_object *obj)
@@ -770,10 +774,14 @@ static int klp_init_object_loaded(struct klp_patch *patch,
struct klp_func *func;
int ret;
+ module_disable_ro(patch->mod);
ret = klp_write_object_relocations(patch->mod, obj);
if (ret)
return ret;
+ arch_klp_init_object_loaded(patch, obj);
+ module_enable_ro(patch->mod);
+
klp_for_each_func(obj, func) {
ret = klp_find_object_symbol(obj->name, func->old_name,
func->old_sympos,
--
2.5.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] livepatch: use arch_klp_init_object_loaded() to finish arch-specific tasks
2016-07-21 5:23 ` [PATCH v2 1/2] livepatch: use arch_klp_init_object_loaded() to finish arch-specific tasks Jessica Yu
@ 2016-08-02 8:25 ` Miroslav Benes
0 siblings, 0 replies; 8+ messages in thread
From: Miroslav Benes @ 2016-08-02 8:25 UTC (permalink / raw)
To: Jessica Yu
Cc: Josh Poimboeuf, Petr Mladek, Jiri Kosina, Chris J Arges,
Eugene Shatokhin, live-patching, x86, linux-kernel
On Thu, 21 Jul 2016, Jessica Yu wrote:
> Introduce arch_klp_init_object_loaded() to complete any additional
> arch-specific tasks during patching. Architecture code may override this
> function.
>
> Signed-off-by: Jessica Yu <jeyu@redhat.com>
> ---
> include/linux/livepatch.h | 3 +++
> kernel/livepatch/core.c | 12 ++++++++++--
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index a93a0b2..9072f04 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -116,6 +116,9 @@ int klp_unregister_patch(struct klp_patch *);
> int klp_enable_patch(struct klp_patch *);
> int klp_disable_patch(struct klp_patch *);
>
> +void arch_klp_init_object_loaded(struct klp_patch *patch,
> + struct klp_object *obj);
> +
> /* Called from the module loader during module coming/going states */
> int klp_module_coming(struct module *mod);
> void klp_module_going(struct module *mod);
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 5c2bc10..164eff6 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -274,7 +274,6 @@ static int klp_write_object_relocations(struct module *pmod,
>
> objname = klp_is_module(obj) ? obj->name : "vmlinux";
>
> - module_disable_ro(pmod);
> /* For each klp relocation section */
> for (i = 1; i < pmod->klp_info->hdr.e_shnum; i++) {
> sec = pmod->klp_info->sechdrs + i;
> @@ -309,7 +308,6 @@ static int klp_write_object_relocations(struct module *pmod,
> break;
> }
>
> - module_enable_ro(pmod);
> return ret;
> }
>
> @@ -763,6 +761,12 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func)
> func->old_sympos ? func->old_sympos : 1);
> }
>
> +/* Arches may override this to finish any remaining arch-specific tasks */
> +void __weak arch_klp_init_object_loaded(struct klp_patch *patch,
> + struct klp_object *obj)
> +{
> +}
> +
> /* parts of the initialization that is done only when the object is loaded */
> static int klp_init_object_loaded(struct klp_patch *patch,
> struct klp_object *obj)
> @@ -770,10 +774,14 @@ static int klp_init_object_loaded(struct klp_patch *patch,
> struct klp_func *func;
> int ret;
>
> + module_disable_ro(patch->mod);
> ret = klp_write_object_relocations(patch->mod, obj);
> if (ret)
> return ret;
We need to call module_enable_ro(patch->mod) here before returning back.
Miroslav
>
> + arch_klp_init_object_loaded(patch, obj);
> + module_enable_ro(patch->mod);
> +
> klp_for_each_func(obj, func) {
> ret = klp_find_object_symbol(obj->name, func->old_name,
> func->old_sympos,
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] livepatch/x86: apply alternatives and paravirt patches after relocations
2016-07-21 5:23 [PATCH v2 0/2] Fix issue with alternatives/paravirt patches Jessica Yu
2016-07-21 5:23 ` [PATCH v2 1/2] livepatch: use arch_klp_init_object_loaded() to finish arch-specific tasks Jessica Yu
@ 2016-07-21 5:24 ` Jessica Yu
2016-08-02 8:59 ` Miroslav Benes
2016-07-26 20:59 ` [PATCH v2 0/2] Fix issue with alternatives/paravirt patches Josh Poimboeuf
2 siblings, 1 reply; 8+ messages in thread
From: Jessica Yu @ 2016-07-21 5:24 UTC (permalink / raw)
To: Josh Poimboeuf, Miroslav Benes, Petr Mladek, Jiri Kosina,
Chris J Arges, Eugene Shatokhin
Cc: live-patching, x86, linux-kernel, Jessica Yu
Implement arch_klp_init_object_loaded() for x86, which applies
alternatives/paravirt patches. This fixes the order in which relocations
and alternatives/paravirt patches are applied.
Previously, if a patch module had alternatives or paravirt patches,
these were applied first by the module loader before livepatch can apply
per-object relocations. The (buggy) sequence of events was:
(1) Load patch module
(2) Apply alternatives and paravirt patches to patch module
* Note that these are applied to the new functions in the patch module
(3) Apply per-object relocations to patch module when target module loads.
* This clobbers what was written in step 2
This lead to crashes and corruption in general, since livepatch would
overwrite or step on previously applied alternative/paravirt patches.
The correct sequence of events should be:
(1) Load patch module
(2) Apply per-object relocations to patch module
(3) Apply alternatives and paravirt patches to patch module
This is fixed by delaying paravirt/alternatives patching until after
relocations are applied. Any .altinstructions or .parainstructions sections in
a patch module are prefixed with ".klp.arch.${objname}" and applied in
arch_klp_init_object_loaded().
Signed-off-by: Jessica Yu <jeyu@redhat.com>
---
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/livepatch.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+)
create mode 100644 arch/x86/kernel/livepatch.c
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 0503f5b..4f656fe 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_X86_MPPARSE) += mpparse.o
obj-y += apic/
obj-$(CONFIG_X86_REBOOTFIXUPS) += reboot_fixups_32.o
obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.o
obj-$(CONFIG_X86_TSC) += trace_clock.o
diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c
new file mode 100644
index 0000000..78209f1
--- /dev/null
+++ b/arch/x86/kernel/livepatch.c
@@ -0,0 +1,65 @@
+/*
+ * livepatch.c - x86-specific Kernel Live Patching Core
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/module.h>
+#include <linux/kallsyms.h>
+#include <linux/livepatch.h>
+#include <asm/text-patching.h>
+
+/* Apply per-object alternatives. Based on x86 module_finalize() */
+void arch_klp_init_object_loaded(struct klp_patch *patch,
+ struct klp_object *obj)
+{
+ int cnt;
+ struct klp_modinfo *info;
+ Elf_Shdr *s, *alt = NULL, *para = NULL;
+ void *aseg, *pseg;
+ const char *objname;
+ char sec_objname[MODULE_NAME_LEN];
+ char secname[KSYM_NAME_LEN];
+
+ info = patch->mod->klp_info;
+ objname = obj->name ? obj->name : "vmlinux";
+
+ /* See livepatch core code for BUILD_BUG_ON() explanation */
+ BUILD_BUG_ON(MODULE_NAME_LEN < 56 || KSYM_NAME_LEN != 128);
+
+ for (s = info->sechdrs; s < info->sechdrs + info->hdr.e_shnum; s++) {
+ /* Apply per-object .klp.arch sections */
+ cnt = sscanf(info->secstrings + s->sh_name,
+ ".klp.arch.%55[^.].%127s",
+ sec_objname, secname);
+ if (cnt != 2)
+ continue;
+ if (strcmp(sec_objname, objname))
+ continue;
+ if (!strcmp("altinstructions", secname))
+ alt = s;
+ if (!strcmp("parainstructions", secname))
+ para = s;
+ }
+
+ if (alt) {
+ aseg = (void *) alt->sh_addr;
+ apply_alternatives(aseg, aseg + alt->sh_size);
+ }
+
+ if (para) {
+ pseg = (void *) para->sh_addr;
+ apply_paravirt(pseg, pseg + para->sh_size);
+ }
+}
--
2.5.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] livepatch/x86: apply alternatives and paravirt patches after relocations
2016-07-21 5:24 ` [PATCH v2 2/2] livepatch/x86: apply alternatives and paravirt patches after relocations Jessica Yu
@ 2016-08-02 8:59 ` Miroslav Benes
2016-08-02 12:59 ` Petr Mladek
2016-08-02 13:02 ` Jiri Kosina
0 siblings, 2 replies; 8+ messages in thread
From: Miroslav Benes @ 2016-08-02 8:59 UTC (permalink / raw)
To: Jessica Yu
Cc: Josh Poimboeuf, Petr Mladek, Jiri Kosina, Chris J Arges,
Eugene Shatokhin, live-patching, x86, linux-kernel
On Thu, 21 Jul 2016, Jessica Yu wrote:
> Implement arch_klp_init_object_loaded() for x86, which applies
> alternatives/paravirt patches. This fixes the order in which relocations
> and alternatives/paravirt patches are applied.
>
> Previously, if a patch module had alternatives or paravirt patches,
> these were applied first by the module loader before livepatch can apply
> per-object relocations. The (buggy) sequence of events was:
>
> (1) Load patch module
> (2) Apply alternatives and paravirt patches to patch module
> * Note that these are applied to the new functions in the patch module
> (3) Apply per-object relocations to patch module when target module loads.
> * This clobbers what was written in step 2
>
> This lead to crashes and corruption in general, since livepatch would
> overwrite or step on previously applied alternative/paravirt patches.
> The correct sequence of events should be:
>
> (1) Load patch module
> (2) Apply per-object relocations to patch module
> (3) Apply alternatives and paravirt patches to patch module
>
> This is fixed by delaying paravirt/alternatives patching until after
> relocations are applied. Any .altinstructions or .parainstructions sections in
> a patch module are prefixed with ".klp.arch.${objname}" and applied in
> arch_klp_init_object_loaded().
Would it make sense to update our documentation and include this info?
> Signed-off-by: Jessica Yu <jeyu@redhat.com>
> ---
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/livepatch.c | 65 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+)
> create mode 100644 arch/x86/kernel/livepatch.c
>
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 0503f5b..4f656fe 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_X86_MPPARSE) += mpparse.o
> obj-y += apic/
> obj-$(CONFIG_X86_REBOOTFIXUPS) += reboot_fixups_32.o
> obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o
> +obj-$(CONFIG_LIVEPATCH) += livepatch.o
> obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
> obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.o
> obj-$(CONFIG_X86_TSC) += trace_clock.o
> diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c
> new file mode 100644
> index 0000000..78209f1
> --- /dev/null
> +++ b/arch/x86/kernel/livepatch.c
> @@ -0,0 +1,65 @@
> +/*
> + * livepatch.c - x86-specific Kernel Live Patching Core
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kallsyms.h>
> +#include <linux/livepatch.h>
> +#include <asm/text-patching.h>
It seems I don't have such header file here and it causes a build error.
Shouldn't it be <asm/alternative.h>?
> +
> +/* Apply per-object alternatives. Based on x86 module_finalize() */
> +void arch_klp_init_object_loaded(struct klp_patch *patch,
> + struct klp_object *obj)
> +{
> + int cnt;
> + struct klp_modinfo *info;
> + Elf_Shdr *s, *alt = NULL, *para = NULL;
> + void *aseg, *pseg;
> + const char *objname;
> + char sec_objname[MODULE_NAME_LEN];
> + char secname[KSYM_NAME_LEN];
> +
> + info = patch->mod->klp_info;
> + objname = obj->name ? obj->name : "vmlinux";
> +
> + /* See livepatch core code for BUILD_BUG_ON() explanation */
> + BUILD_BUG_ON(MODULE_NAME_LEN < 56 || KSYM_NAME_LEN != 128);
> +
> + for (s = info->sechdrs; s < info->sechdrs + info->hdr.e_shnum; s++) {
> + /* Apply per-object .klp.arch sections */
> + cnt = sscanf(info->secstrings + s->sh_name,
> + ".klp.arch.%55[^.].%127s",
> + sec_objname, secname);
> + if (cnt != 2)
> + continue;
> + if (strcmp(sec_objname, objname))
> + continue;
> + if (!strcmp("altinstructions", secname))
> + alt = s;
> + if (!strcmp("parainstructions", secname))
> + para = s;
> + }
> +
> + if (alt) {
> + aseg = (void *) alt->sh_addr;
> + apply_alternatives(aseg, aseg + alt->sh_size);
> + }
> +
> + if (para) {
> + pseg = (void *) para->sh_addr;
> + apply_paravirt(pseg, pseg + para->sh_size);
> + }
> +}
Otherwise it looks good.
Regards,
Miroslav
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] livepatch/x86: apply alternatives and paravirt patches after relocations
2016-08-02 8:59 ` Miroslav Benes
@ 2016-08-02 12:59 ` Petr Mladek
2016-08-02 13:02 ` Jiri Kosina
1 sibling, 0 replies; 8+ messages in thread
From: Petr Mladek @ 2016-08-02 12:59 UTC (permalink / raw)
To: Miroslav Benes
Cc: Jessica Yu, Josh Poimboeuf, Jiri Kosina, Chris J Arges,
Eugene Shatokhin, live-patching, x86, linux-kernel
On Tue 2016-08-02 10:59:07, Miroslav Benes wrote:
> On Thu, 21 Jul 2016, Jessica Yu wrote:
>
> > Implement arch_klp_init_object_loaded() for x86, which applies
> > alternatives/paravirt patches. This fixes the order in which relocations
> > and alternatives/paravirt patches are applied.
> >
> > diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c
> > new file mode 100644
> > index 0000000..78209f1
> > --- /dev/null
> > +++ b/arch/x86/kernel/livepatch.c
> > +#include <linux/module.h>
> > +#include <linux/kallsyms.h>
> > +#include <linux/livepatch.h>
> > +#include <asm/text-patching.h>
>
> It seems I don't have such header file here and it causes a build error.
> Shouldn't it be <asm/alternative.h>?
It is in linux-next, see 35de5b0692aaa ("x86/asm: Stop depending on
ptrace.h in alternative.h").
I agree with the other Mirek's comments. Otherwise, it looks good.
Thanks for working on it,
Petr
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] livepatch/x86: apply alternatives and paravirt patches after relocations
2016-08-02 8:59 ` Miroslav Benes
2016-08-02 12:59 ` Petr Mladek
@ 2016-08-02 13:02 ` Jiri Kosina
1 sibling, 0 replies; 8+ messages in thread
From: Jiri Kosina @ 2016-08-02 13:02 UTC (permalink / raw)
To: Miroslav Benes
Cc: Jessica Yu, Josh Poimboeuf, Petr Mladek, Chris J Arges,
Eugene Shatokhin, live-patching, x86, linux-kernel
On Tue, 2 Aug 2016, Miroslav Benes wrote:
> > +#include <linux/module.h>
> > +#include <linux/kallsyms.h>
> > +#include <linux/livepatch.h>
> > +#include <asm/text-patching.h>
>
> It seems I don't have such header file here and it causes a build error.
> Shouldn't it be <asm/alternative.h>?
I've now refreshed our for-next branch so that it's based on v4.7 to allow
for it being used as a up-to-date basis for development.
(there have been no KLP patches this merge window, which made for-next
also quite silent).
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] Fix issue with alternatives/paravirt patches
2016-07-21 5:23 [PATCH v2 0/2] Fix issue with alternatives/paravirt patches Jessica Yu
2016-07-21 5:23 ` [PATCH v2 1/2] livepatch: use arch_klp_init_object_loaded() to finish arch-specific tasks Jessica Yu
2016-07-21 5:24 ` [PATCH v2 2/2] livepatch/x86: apply alternatives and paravirt patches after relocations Jessica Yu
@ 2016-07-26 20:59 ` Josh Poimboeuf
2 siblings, 0 replies; 8+ messages in thread
From: Josh Poimboeuf @ 2016-07-26 20:59 UTC (permalink / raw)
To: Jessica Yu
Cc: Miroslav Benes, Petr Mladek, Jiri Kosina, Chris J Arges,
Eugene Shatokhin, live-patching, x86, linux-kernel
On Thu, Jul 21, 2016 at 01:23:58AM -0400, Jessica Yu wrote:
> Hi,
>
> A few months ago, Chris Arges reported a bug involving alternatives/paravirt
> patching that was discussed here [1] and here [2]. To briefly summarize the
> bug, patch modules that contained .altinstructions or .parainstructions
> sections would break because these alternative/paravirt patches would be
> applied first by the module loader (see x86 module_finalize()), then
> livepatch would later clobber these patches when applying per-object
> relocations. This lead to crashes and unpredictable behavior.
>
> One conclusion we reached from our last discussion was that we will
> need to introduce some arch-specific code to address this problem.
> This patchset presents a possible fix for the bug by adding a new
> arch-specific arch_klp_init_object_loaded() function that by default
> does nothing but can be overridden by different arches.
>
> To fix this issue for x86, since we can access a patch module's Elf
> sections through mod->klp_info, we can simply delay the calls to
> apply_paravirt() and apply_alternatives() to arch_klp_init_object_loaded(),
> which is called after relocations have been written for an object.
> In addition, for patch modules, .parainstructions and .altinstructions are
> prefixed by ".klp.arch.${objname}" so that the module loader ignores them
> and livepatch can apply them manually.
>
> Currently for kpatch, we don't support including jump table sections in
> the patch module, and supporting .smp_locks is currently broken, so we
> don't consider those sections (for now).
>
> I did some light testing with some patches to kvm and verified that the
> original issue reported in [2] was fixed.
>
> Based on linux-next.
For the series:
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
--
Josh
^ permalink raw reply [flat|nested] 8+ messages in thread