| Message ID | cover.1602708025.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | kasan: hardware tag-based mode for production use on arm64 | expand |
On Wed, 14 Oct 2020 at 22:44, Andrey Konovalov <andreyknvl@google.com> wrote: > This patchset is not complete (see particular TODOs in the last patch), > and I haven't performed any benchmarking yet, but I would like to start the > discussion now and hear people's opinions regarding the questions mentioned > below. > > === Overview > > This patchset adopts the existing hardware tag-based KASAN mode [1] for > use in production as a memory corruption mitigation. Hardware tag-based > KASAN relies on arm64 Memory Tagging Extension (MTE) [2] to perform memory > and pointer tagging. Please see [3] and [4] for detailed analysis of how > MTE helps to fight memory safety problems. > > The current plan is reuse CONFIG_KASAN_HW_TAGS for production, but add a > boot time switch, that allows to choose between a debugging mode, that > includes all KASAN features as they are, and a production mode, that only > includes the essentials like tag checking. > > It is essential that switching between these modes doesn't require > rebuilding the kernel with different configs, as this is required by the > Android GKI initiative [5]. > > The last patch of this series adds a new boot time parameter called > kasan_mode, which can have the following values: > > - "kasan_mode=on" - only production features > - "kasan_mode=debug" - all debug features > - "kasan_mode=off" - no checks at all (not implemented yet) > > Currently outlined differences between "on" and "debug": > > - "on" doesn't keep track of alloc/free stacks, and therefore doesn't > require the additional memory to store those > - "on" uses asyncronous tag checking (not implemented yet) > > === Questions > > The intention with this kind of a high level switch is to hide the > implementation details. Arguably, we could add multiple switches that allow > to separately control each KASAN or MTE feature, but I'm not sure there's > much value in that. > > Does this make sense? Any preference regarding the name of the parameter > and its values? KASAN itself used to be a debugging tool only. So introducing an "on" mode which no longer follows this convention may be confusing. Instead, maybe the following might be less confusing: "full" - current "debug", normal KASAN, all debugging help available. "opt" - current "on", optimized mode for production. "on" - automatic selection => chooses "full" if CONFIG_DEBUG_KERNEL, "opt" otherwise. "off" - as before. Also, if there is no other kernel boot parameter named "kasan" yet, maybe it could just be "kasan=..." ? > What should be the default when the parameter is not specified? I would > argue that it should be "debug" (for hardware that supports MTE, otherwise > "off"), as it's the implied default for all other KASAN modes. Perhaps we could make this dependent on CONFIG_DEBUG_KERNEL as above. I do not think that having the full/debug KASAN enabled on production kernels adds any value because for it to be useful requires somebody to actually look at the stacktraces; I think that choice should be made explicitly if it's a production kernel. My guess is that we'll save explaining performance differences and resulting headaches for ourselves and others that way. > Should we somehow control whether to panic the kernel on a tag fault? > Another boot time parameter perhaps? It already respects panic_on_warn, correct? > Any ideas as to how properly estimate the slowdown? As there's no > MTE-enabled hardware yet, the only way to test these patches is use an > emulator (like QEMU). The delay that is added by the emulator (for setting > and checking the tags) is different from the hardware delay, and this skews > the results. > > A question to KASAN maintainers: what would be the best way to support the > "off" mode? I see two potential approaches: add a check into each kasan > callback (easier to implement, but we still call kasan callbacks, even > though they immediately return), or add inline header wrappers that do the > same. [...] Thanks, -- Marco
On Thu, Oct 15, 2020 at 4:41 PM Marco Elver <elver@google.com> wrote: > > On Wed, 14 Oct 2020 at 22:44, Andrey Konovalov <andreyknvl@google.com> wrote: > > This patchset is not complete (see particular TODOs in the last patch), > > and I haven't performed any benchmarking yet, but I would like to start the > > discussion now and hear people's opinions regarding the questions mentioned > > below. > > > > === Overview > > > > This patchset adopts the existing hardware tag-based KASAN mode [1] for > > use in production as a memory corruption mitigation. Hardware tag-based > > KASAN relies on arm64 Memory Tagging Extension (MTE) [2] to perform memory > > and pointer tagging. Please see [3] and [4] for detailed analysis of how > > MTE helps to fight memory safety problems. > > > > The current plan is reuse CONFIG_KASAN_HW_TAGS for production, but add a > > boot time switch, that allows to choose between a debugging mode, that > > includes all KASAN features as they are, and a production mode, that only > > includes the essentials like tag checking. > > > > It is essential that switching between these modes doesn't require > > rebuilding the kernel with different configs, as this is required by the > > Android GKI initiative [5]. > > > > The last patch of this series adds a new boot time parameter called > > kasan_mode, which can have the following values: > > > > - "kasan_mode=on" - only production features > > - "kasan_mode=debug" - all debug features > > - "kasan_mode=off" - no checks at all (not implemented yet) > > > > Currently outlined differences between "on" and "debug": > > > > - "on" doesn't keep track of alloc/free stacks, and therefore doesn't > > require the additional memory to store those > > - "on" uses asyncronous tag checking (not implemented yet) > > > > === Questions > > > > The intention with this kind of a high level switch is to hide the > > implementation details. Arguably, we could add multiple switches that allow > > to separately control each KASAN or MTE feature, but I'm not sure there's > > much value in that. > > > > Does this make sense? Any preference regarding the name of the parameter > > and its values? > > KASAN itself used to be a debugging tool only. So introducing an "on" > mode which no longer follows this convention may be confusing. Yeah, perhaps "on" is not the best name here. > Instead, maybe the following might be less confusing: > > "full" - current "debug", normal KASAN, all debugging help available. > "opt" - current "on", optimized mode for production. How about "prod" here? > "on" - automatic selection => chooses "full" if CONFIG_DEBUG_KERNEL, > "opt" otherwise. > "off" - as before. It actually makes sense to depend on CONFIG_DEBUG_KERNEL, I like this idea. > > Also, if there is no other kernel boot parameter named "kasan" yet, > maybe it could just be "kasan=..." ? Sounds good to me too. > > What should be the default when the parameter is not specified? I would > > argue that it should be "debug" (for hardware that supports MTE, otherwise > > "off"), as it's the implied default for all other KASAN modes. > > Perhaps we could make this dependent on CONFIG_DEBUG_KERNEL as above. > I do not think that having the full/debug KASAN enabled on production > kernels adds any value because for it to be useful requires somebody > to actually look at the stacktraces; I think that choice should be > made explicitly if it's a production kernel. My guess is that we'll > save explaining performance differences and resulting headaches for > ourselves and others that way. Ack. > > Should we somehow control whether to panic the kernel on a tag fault? > > Another boot time parameter perhaps? > > It already respects panic_on_warn, correct? Yes, but Android is unlikely to enable panic_on_warn as they have warnings happening all over. AFAIR Pixel 3/4 kernels actually have a custom patch that enables kernel panic for KASAN crashes specifically (even though they don't obviously use KASAN in production), and I think it's better to provide a similar facility upstream. Maybe call it panic_on_kasan or something?
On Fri, 16 Oct 2020 at 15:17, 'Andrey Konovalov' via kasan-dev <kasan-dev@googlegroups.com> wrote: [...] > > > The intention with this kind of a high level switch is to hide the > > > implementation details. Arguably, we could add multiple switches that allow > > > to separately control each KASAN or MTE feature, but I'm not sure there's > > > much value in that. > > > > > > Does this make sense? Any preference regarding the name of the parameter > > > and its values? > > > > KASAN itself used to be a debugging tool only. So introducing an "on" > > mode which no longer follows this convention may be confusing. > > Yeah, perhaps "on" is not the best name here. > > > Instead, maybe the following might be less confusing: > > > > "full" - current "debug", normal KASAN, all debugging help available. > > "opt" - current "on", optimized mode for production. > > How about "prod" here? SGTM. [...] > > > > Should we somehow control whether to panic the kernel on a tag fault? > > > Another boot time parameter perhaps? > > > > It already respects panic_on_warn, correct? > > Yes, but Android is unlikely to enable panic_on_warn as they have > warnings happening all over. AFAIR Pixel 3/4 kernels actually have a > custom patch that enables kernel panic for KASAN crashes specifically > (even though they don't obviously use KASAN in production), and I > think it's better to provide a similar facility upstream. Maybe call > it panic_on_kasan or something? Best would be if kasan= can take another option, e.g. "kasan=prod,panic". I think you can change the strcmp() to a str_has_prefix() for the checks for full/prod/on/off, and then check if what comes after it is ",panic". Thanks, -- Marco
On Wed, Oct 14, 2020 at 10:44 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > This patchset is not complete (see particular TODOs in the last patch), > and I haven't performed any benchmarking yet, but I would like to start the > discussion now and hear people's opinions regarding the questions mentioned > below. > > === Overview > > This patchset adopts the existing hardware tag-based KASAN mode [1] for > use in production as a memory corruption mitigation. Hardware tag-based > KASAN relies on arm64 Memory Tagging Extension (MTE) [2] to perform memory > and pointer tagging. Please see [3] and [4] for detailed analysis of how > MTE helps to fight memory safety problems. > > The current plan is reuse CONFIG_KASAN_HW_TAGS for production, but add a > boot time switch, that allows to choose between a debugging mode, that > includes all KASAN features as they are, and a production mode, that only > includes the essentials like tag checking. > > It is essential that switching between these modes doesn't require > rebuilding the kernel with different configs, as this is required by the > Android GKI initiative [5]. > > The last patch of this series adds a new boot time parameter called > kasan_mode, which can have the following values: > > - "kasan_mode=on" - only production features > - "kasan_mode=debug" - all debug features > - "kasan_mode=off" - no checks at all (not implemented yet) > > Currently outlined differences between "on" and "debug": > > - "on" doesn't keep track of alloc/free stacks, and therefore doesn't > require the additional memory to store those > - "on" uses asyncronous tag checking (not implemented yet) > > === Questions > > The intention with this kind of a high level switch is to hide the > implementation details. Arguably, we could add multiple switches that allow > to separately control each KASAN or MTE feature, but I'm not sure there's > much value in that. > > Does this make sense? Any preference regarding the name of the parameter > and its values? > > What should be the default when the parameter is not specified? I would > argue that it should be "debug" (for hardware that supports MTE, otherwise > "off"), as it's the implied default for all other KASAN modes. > > Should we somehow control whether to panic the kernel on a tag fault? > Another boot time parameter perhaps? > > Any ideas as to how properly estimate the slowdown? As there's no > MTE-enabled hardware yet, the only way to test these patches is use an > emulator (like QEMU). The delay that is added by the emulator (for setting > and checking the tags) is different from the hardware delay, and this skews > the results. > > A question to KASAN maintainers: what would be the best way to support the > "off" mode? I see two potential approaches: add a check into each kasan > callback (easier to implement, but we still call kasan callbacks, even > though they immediately return), or add inline header wrappers that do the > same. CC Kostya and Serban. > > === Notes > > This patchset is available here: > > https://github.com/xairy/linux/tree/up-prod-mte-rfc1 > > and on Gerrit here: > > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/3460 > > This patchset is based on v5 of "kasan: add hardware tag-based mode for > arm64" patchset [1]. > > For testing in QEMU hardware tag-based KASAN requires: > > 1. QEMU built from master [6] (use "-machine virt,mte=on -cpu max" arguments > to run). > 2. GCC version 10. > > [1] https://lore.kernel.org/linux-arm-kernel/cover.1602535397.git.andreyknvl@google.com/ > [2] https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/enhancing-memory-safety > [3] https://arxiv.org/pdf/1802.09517.pdf > [4] https://github.com/microsoft/MSRC-Security-Research/blob/master/papers/2020/Security%20analysis%20of%20memory%20tagging.pdf > [5] https://source.android.com/devices/architecture/kernel/generic-kernel-image > [6] https://github.com/qemu/qemu > > Andrey Konovalov (8): > kasan: simplify quarantine_put call > kasan: rename get_alloc/free_info > kasan: introduce set_alloc_info > kasan: unpoison stack only with CONFIG_KASAN_STACK > kasan: mark kasan_init_tags as __init > kasan, arm64: move initialization message > arm64: kasan: Add system_supports_tags helper > kasan: add and integrate kasan_mode boot param > > arch/arm64/include/asm/memory.h | 1 + > arch/arm64/kernel/sleep.S | 2 +- > arch/arm64/mm/kasan_init.c | 3 ++ > arch/x86/kernel/acpi/wakeup_64.S | 2 +- > include/linux/kasan.h | 14 ++--- > mm/kasan/common.c | 90 ++++++++++++++++++-------------- > mm/kasan/generic.c | 18 ++++--- > mm/kasan/hw_tags.c | 63 ++++++++++++++++++++-- > mm/kasan/kasan.h | 25 ++++++--- > mm/kasan/quarantine.c | 5 +- > mm/kasan/report.c | 22 +++++--- > mm/kasan/report_sw_tags.c | 2 +- > mm/kasan/sw_tags.c | 14 +++-- > 13 files changed, 182 insertions(+), 79 deletions(-) > > -- > 2.28.0.1011.ga647a8990f-goog >
On Thu, Oct 15, 2020 at 4:41 PM Marco Elver <elver@google.com> wrote: > > On Wed, 14 Oct 2020 at 22:44, Andrey Konovalov <andreyknvl@google.com> wrote: > > This patchset is not complete (see particular TODOs in the last patch), > > and I haven't performed any benchmarking yet, but I would like to start the > > discussion now and hear people's opinions regarding the questions mentioned > > below. > > > > === Overview > > > > This patchset adopts the existing hardware tag-based KASAN mode [1] for > > use in production as a memory corruption mitigation. Hardware tag-based > > KASAN relies on arm64 Memory Tagging Extension (MTE) [2] to perform memory > > and pointer tagging. Please see [3] and [4] for detailed analysis of how > > MTE helps to fight memory safety problems. > > > > The current plan is reuse CONFIG_KASAN_HW_TAGS for production, but add a > > boot time switch, that allows to choose between a debugging mode, that > > includes all KASAN features as they are, and a production mode, that only > > includes the essentials like tag checking. > > > > It is essential that switching between these modes doesn't require > > rebuilding the kernel with different configs, as this is required by the > > Android GKI initiative [5]. > > > > The last patch of this series adds a new boot time parameter called > > kasan_mode, which can have the following values: > > > > - "kasan_mode=on" - only production features > > - "kasan_mode=debug" - all debug features > > - "kasan_mode=off" - no checks at all (not implemented yet) > > > > Currently outlined differences between "on" and "debug": > > > > - "on" doesn't keep track of alloc/free stacks, and therefore doesn't > > require the additional memory to store those > > - "on" uses asyncronous tag checking (not implemented yet) > > > > === Questions > > > > The intention with this kind of a high level switch is to hide the > > implementation details. Arguably, we could add multiple switches that allow > > to separately control each KASAN or MTE feature, but I'm not sure there's > > much value in that. > > > > Does this make sense? Any preference regarding the name of the parameter > > and its values? > > KASAN itself used to be a debugging tool only. So introducing an "on" > mode which no longer follows this convention may be confusing. > Instead, maybe the following might be less confusing: > > "full" - current "debug", normal KASAN, all debugging help available. > "opt" - current "on", optimized mode for production. > "on" - automatic selection => chooses "full" if CONFIG_DEBUG_KERNEL, > "opt" otherwise. > "off" - as before. > > Also, if there is no other kernel boot parameter named "kasan" yet, > maybe it could just be "kasan=..." ? > > > What should be the default when the parameter is not specified? I would > > argue that it should be "debug" (for hardware that supports MTE, otherwise > > "off"), as it's the implied default for all other KASAN modes. > > Perhaps we could make this dependent on CONFIG_DEBUG_KERNEL as above. > I do not think that having the full/debug KASAN enabled on production > kernels adds any value because for it to be useful requires somebody > to actually look at the stacktraces; I think that choice should be > made explicitly if it's a production kernel. My guess is that we'll > save explaining performance differences and resulting headaches for > ourselves and others that way. > > > Should we somehow control whether to panic the kernel on a tag fault? > > Another boot time parameter perhaps? > > It already respects panic_on_warn, correct? > > > Any ideas as to how properly estimate the slowdown? As there's no > > MTE-enabled hardware yet, the only way to test these patches is use an > > emulator (like QEMU). The delay that is added by the emulator (for setting > > and checking the tags) is different from the hardware delay, and this skews > > the results. > > > > A question to KASAN maintainers: what would be the best way to support the > > "off" mode? I see two potential approaches: add a check into each kasan > > callback (easier to implement, but we still call kasan callbacks, even > > though they immediately return), or add inline header wrappers that do the > > same. > [...] CC Kostya and Serban.
On Fri, Oct 16, 2020 at 3:31 PM Marco Elver <elver@google.com> wrote: > > On Fri, 16 Oct 2020 at 15:17, 'Andrey Konovalov' via kasan-dev > <kasan-dev@googlegroups.com> wrote: > [...] > > > > The intention with this kind of a high level switch is to hide the > > > > implementation details. Arguably, we could add multiple switches that allow > > > > to separately control each KASAN or MTE feature, but I'm not sure there's > > > > much value in that. > > > > > > > > Does this make sense? Any preference regarding the name of the parameter > > > > and its values? > > > > > > KASAN itself used to be a debugging tool only. So introducing an "on" > > > mode which no longer follows this convention may be confusing. > > > > Yeah, perhaps "on" is not the best name here. > > > > > Instead, maybe the following might be less confusing: > > > > > > "full" - current "debug", normal KASAN, all debugging help available. > > > "opt" - current "on", optimized mode for production. > > > > How about "prod" here? > > SGTM. > > [...] > > > > > > Should we somehow control whether to panic the kernel on a tag fault? > > > > Another boot time parameter perhaps? > > > > > > It already respects panic_on_warn, correct? > > > > Yes, but Android is unlikely to enable panic_on_warn as they have > > warnings happening all over. AFAIR Pixel 3/4 kernels actually have a > > custom patch that enables kernel panic for KASAN crashes specifically > > (even though they don't obviously use KASAN in production), and I > > think it's better to provide a similar facility upstream. Maybe call > > it panic_on_kasan or something? > > Best would be if kasan= can take another option, e.g. > "kasan=prod,panic". I think you can change the strcmp() to a > str_has_prefix() for the checks for full/prod/on/off, and then check > if what comes after it is ",panic". > > Thanks, > -- Marco CC Kostya and Serban.
On Wed, 14 Oct 2020 at 22:44, Andrey Konovalov <andreyknvl@google.com> wrote: [...] > A question to KASAN maintainers: what would be the best way to support the > "off" mode? I see two potential approaches: add a check into each kasan > callback (easier to implement, but we still call kasan callbacks, even > though they immediately return), or add inline header wrappers that do the > same. This is tricky, because we don't know how bad the performance will be if we keep them as calls. We'd have to understand the performance impact of keeping them as calls, and if the performance impact is acceptable or not. Without understanding the performance impact, the only viable option I see is to add __always_inline kasan_foo() wrappers, which use the static branch to guard calls to __kasan_foo(). Thanks, -- Marco
Hi, I would like to hear opinions from others in CC on these choices: * Production use of In-kernel MTE should be based on stripped-down KASAN, or implemented independently? * Should we aim at a single boot-time flag (with several values) or for several independent flags (OFF/SYNC/ASYNC, Stack traces on/off) Andrey, please give us some idea of the CPU and RAM overheads other than those coming from MTE * stack trace collection and storage * adding redzones to every allocation - not strictly needed for MTE, but convenient to store the stack trace IDs. Andrey: with production MTE we should not be using quarantine, which means storing the stack trace IDs in the deallocated memory doesn't provide good report quality. We may need to consider another approach, e.g. the one used in HWASAN (separate ring buffer, per thread or per core) --kcc On Fri, Oct 16, 2020 at 8:52 AM Andrey Konovalov <andreyknvl@google.com> wrote: > > On Fri, Oct 16, 2020 at 3:31 PM Marco Elver <elver@google.com> wrote: > > > > On Fri, 16 Oct 2020 at 15:17, 'Andrey Konovalov' via kasan-dev > > <kasan-dev@googlegroups.com> wrote: > > [...] > > > > > The intention with this kind of a high level switch is to hide the > > > > > implementation details. Arguably, we could add multiple switches that allow > > > > > to separately control each KASAN or MTE feature, but I'm not sure there's > > > > > much value in that. > > > > > > > > > > Does this make sense? Any preference regarding the name of the parameter > > > > > and its values? > > > > > > > > KASAN itself used to be a debugging tool only. So introducing an "on" > > > > mode which no longer follows this convention may be confusing. > > > > > > Yeah, perhaps "on" is not the best name here. > > > > > > > Instead, maybe the following might be less confusing: > > > > > > > > "full" - current "debug", normal KASAN, all debugging help available. > > > > "opt" - current "on", optimized mode for production. > > > > > > How about "prod" here? > > > > SGTM. > > > > [...] > > > > > > > > Should we somehow control whether to panic the kernel on a tag fault? > > > > > Another boot time parameter perhaps? > > > > > > > > It already respects panic_on_warn, correct? > > > > > > Yes, but Android is unlikely to enable panic_on_warn as they have > > > warnings happening all over. AFAIR Pixel 3/4 kernels actually have a > > > custom patch that enables kernel panic for KASAN crashes specifically > > > (even though they don't obviously use KASAN in production), and I > > > think it's better to provide a similar facility upstream. Maybe call > > > it panic_on_kasan or something? > > > > Best would be if kasan= can take another option, e.g. > > "kasan=prod,panic". I think you can change the strcmp() to a > > str_has_prefix() for the checks for full/prod/on/off, and then check > > if what comes after it is ",panic". > > > > Thanks, > > -- Marco > > CC Kostya and Serban.
On Mon, Oct 19, 2020 at 2:23 PM Marco Elver <elver@google.com> wrote: > > On Wed, 14 Oct 2020 at 22:44, Andrey Konovalov <andreyknvl@google.com> wrote: > [...] > > A question to KASAN maintainers: what would be the best way to support the > > "off" mode? I see two potential approaches: add a check into each kasan > > callback (easier to implement, but we still call kasan callbacks, even > > though they immediately return), or add inline header wrappers that do the > > same. > > This is tricky, because we don't know how bad the performance will be > if we keep them as calls. We'd have to understand the performance > impact of keeping them as calls, and if the performance impact is > acceptable or not. > > Without understanding the performance impact, the only viable option I > see is to add __always_inline kasan_foo() wrappers, which use the > static branch to guard calls to __kasan_foo(). This sounds reasonable to me.
On Tue, Oct 20, 2020 at 12:51 AM Kostya Serebryany <kcc@google.com> wrote: > > Hi, > I would like to hear opinions from others in CC on these choices: > * Production use of In-kernel MTE should be based on stripped-down > KASAN, or implemented independently? Andrey, what are the fundamental consequences of basing MTE on KASAN? I would assume that there are none as we can change KASAN code and special case some code paths as necessary. > * Should we aim at a single boot-time flag (with several values) or > for several independent flags (OFF/SYNC/ASYNC, Stack traces on/off) We won't be able to answer this question for several years until we have actual hardware/users... It's definitely safer to aim at multiple options. I would reuse the fs opt parsing code as we seem to have lots of potential things to configure so that we can do: kasan_options=quarantine=off,fault=panic,trap=async I am also always confused by the term "debug" when configuring the kernel. In some cases it's for debugging of the subsystem (for developers of KASAN), in some cases it adds additional checks to catch misuses of the subsystem. in some - it just adds more debugging output on console. And in this case it's actually neither of these. But I am not sure what's a better name ("full"?). Even if we split options into multiple, we still can have some kind of presents that just flip all other options into reasonable values. > Andrey, please give us some idea of the CPU and RAM overheads other > than those coming from MTE > * stack trace collection and storage > * adding redzones to every allocation - not strictly needed for MTE, > but convenient to store the stack trace IDs. > > Andrey: with production MTE we should not be using quarantine, which > means storing the stack trace IDs > in the deallocated memory doesn't provide good report quality. > We may need to consider another approach, e.g. the one used in HWASAN > (separate ring buffer, per thread or per core) > > --kcc > > > On Fri, Oct 16, 2020 at 8:52 AM Andrey Konovalov <andreyknvl@google.com> wrote: > > > > On Fri, Oct 16, 2020 at 3:31 PM Marco Elver <elver@google.com> wrote: > > > > > > On Fri, 16 Oct 2020 at 15:17, 'Andrey Konovalov' via kasan-dev > > > <kasan-dev@googlegroups.com> wrote: > > > [...] > > > > > > The intention with this kind of a high level switch is to hide the > > > > > > implementation details. Arguably, we could add multiple switches that allow > > > > > > to separately control each KASAN or MTE feature, but I'm not sure there's > > > > > > much value in that. > > > > > > > > > > > > Does this make sense? Any preference regarding the name of the parameter > > > > > > and its values? > > > > > > > > > > KASAN itself used to be a debugging tool only. So introducing an "on" > > > > > mode which no longer follows this convention may be confusing. > > > > > > > > Yeah, perhaps "on" is not the best name here. > > > > > > > > > Instead, maybe the following might be less confusing: > > > > > > > > > > "full" - current "debug", normal KASAN, all debugging help available. > > > > > "opt" - current "on", optimized mode for production. > > > > > > > > How about "prod" here? > > > > > > SGTM. > > > > > > [...] > > > > > > > > > > Should we somehow control whether to panic the kernel on a tag fault? > > > > > > Another boot time parameter perhaps? > > > > > > > > > > It already respects panic_on_warn, correct? > > > > > > > > Yes, but Android is unlikely to enable panic_on_warn as they have > > > > warnings happening all over. AFAIR Pixel 3/4 kernels actually have a > > > > custom patch that enables kernel panic for KASAN crashes specifically > > > > (even though they don't obviously use KASAN in production), and I > > > > think it's better to provide a similar facility upstream. Maybe call > > > > it panic_on_kasan or something? > > > > > > Best would be if kasan= can take another option, e.g. > > > "kasan=prod,panic". I think you can change the strcmp() to a > > > str_has_prefix() for the checks for full/prod/on/off, and then check > > > if what comes after it is ",panic". > > > > > > Thanks, > > > -- Marco > > > > CC Kostya and Serban.
On Tue, Oct 20, 2020 at 7:34 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Tue, Oct 20, 2020 at 12:51 AM Kostya Serebryany <kcc@google.com> wrote: > > > > Hi, > > I would like to hear opinions from others in CC on these choices: > > * Production use of In-kernel MTE should be based on stripped-down > > KASAN, or implemented independently? > > Andrey, what are the fundamental consequences of basing MTE on KASAN? > I would assume that there are none as we can change KASAN code and > special case some code paths as necessary. The main consequence is psychological and manifests in inheriting the name :) But generally you're right. As we can change KASAN code, we can do whatever we want, like adding fast paths for MTE, etc. If we Ctrl+C Ctrl+V KASAN common code, we could potentially do some micro optimizations (like avoiding a couple of checks), but I doubt that will make any difference. > > * Should we aim at a single boot-time flag (with several values) or > > for several independent flags (OFF/SYNC/ASYNC, Stack traces on/off) > > We won't be able to answer this question for several years until we > have actual hardware/users... > It's definitely safer to aim at multiple options. I would reuse the fs > opt parsing code as we seem to have lots of potential things to > configure so that we can do: > kasan_options=quarantine=off,fault=panic,trap=async > > I am also always confused by the term "debug" when configuring the > kernel. In some cases it's for debugging of the subsystem (for > developers of KASAN), in some cases it adds additional checks to catch > misuses of the subsystem. in some - it just adds more debugging output > on console. And in this case it's actually neither of these. But I am > not sure what's a better name ("full"?). Even if we split options into > multiple, we still can have some kind of presents that just flip all > other options into reasonable values. OK, let me try to incorporate the feedback I've heard so far into the next version. > > > Andrey, please give us some idea of the CPU and RAM overheads other > > than those coming from MTE > > * stack trace collection and storage > > * adding redzones to every allocation - not strictly needed for MTE, > > but convenient to store the stack trace IDs. > > > > Andrey: with production MTE we should not be using quarantine, which > > means storing the stack trace IDs > > in the deallocated memory doesn't provide good report quality. > > We may need to consider another approach, e.g. the one used in HWASAN > > (separate ring buffer, per thread or per core) My current priority is cleaning up the mode where stack traces are disabled and estimating the slowdown from KASAN callbacks. Once done with that, I'll switch to these ones.