| Message ID | 20210211194258.4137998-1-nathan@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute | expand |
Did this happen to get picked up already? EOM On Thu, Feb 11, 2021 at 11:43 AM Nathan Chancellor <nathan@kernel.org> wrote: > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > which violates clang's CFI checking because fw_cfg_showrev()'s second > parameter is 'struct attribute', whereas the ->show() member of 'struct > kobj_structure' expects the second parameter to be of type 'struct > kobj_attribute'. > > $ cat /sys/firmware/qemu_fw_cfg/rev > 3 > > $ dmesg | grep "CFI failure" > [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8): > > Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where > this would have been caught automatically by the incompatible pointer > types compiler warning. Update fw_cfg_showrev() accordingly. > > Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") > Link: https://github.com/ClangBuiltLinux/linux/issues/1299 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > --- > drivers/firmware/qemu_fw_cfg.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c > index 0078260fbabe..172c751a4f6c 100644 > --- a/drivers/firmware/qemu_fw_cfg.c > +++ b/drivers/firmware/qemu_fw_cfg.c > @@ -299,15 +299,13 @@ static int fw_cfg_do_platform_probe(struct platform_device *pdev) > return 0; > } > > -static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char *buf) > +static ssize_t fw_cfg_showrev(struct kobject *k, struct kobj_attribute *a, > + char *buf) > { > return sprintf(buf, "%u\n", fw_cfg_rev); > } > > -static const struct { > - struct attribute attr; > - ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf); > -} fw_cfg_rev_attr = { > +static const struct kobj_attribute fw_cfg_rev_attr = { > .attr = { .name = "rev", .mode = S_IRUSR }, > .show = fw_cfg_showrev, > }; > > base-commit: 92bf22614b21a2706f4993b278017e437f7785b3 > -- > 2.30.1 > > -- > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210211194258.4137998-1-nathan%40kernel.org.
On Mon, Feb 22, 2021 at 11:02:34AM -0800, Nick Desaulniers wrote: > Did this happen to get picked up already? EOM I have not gotten an email saying it has been picked up nor does it appear to be in -next. Cheers, Nathan > On Thu, Feb 11, 2021 at 11:43 AM Nathan Chancellor <nathan@kernel.org> wrote: > > > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > > which violates clang's CFI checking because fw_cfg_showrev()'s second > > parameter is 'struct attribute', whereas the ->show() member of 'struct > > kobj_structure' expects the second parameter to be of type 'struct > > kobj_attribute'. > > > > $ cat /sys/firmware/qemu_fw_cfg/rev > > 3 > > > > $ dmesg | grep "CFI failure" > > [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8): > > > > Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where > > this would have been caught automatically by the incompatible pointer > > types compiler warning. Update fw_cfg_showrev() accordingly. > > > > Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") > > Link: https://github.com/ClangBuiltLinux/linux/issues/1299 > > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > > --- > > drivers/firmware/qemu_fw_cfg.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c > > index 0078260fbabe..172c751a4f6c 100644 > > --- a/drivers/firmware/qemu_fw_cfg.c > > +++ b/drivers/firmware/qemu_fw_cfg.c > > @@ -299,15 +299,13 @@ static int fw_cfg_do_platform_probe(struct platform_device *pdev) > > return 0; > > } > > > > -static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char *buf) > > +static ssize_t fw_cfg_showrev(struct kobject *k, struct kobj_attribute *a, > > + char *buf) > > { > > return sprintf(buf, "%u\n", fw_cfg_rev); > > } > > > > -static const struct { > > - struct attribute attr; > > - ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf); > > -} fw_cfg_rev_attr = { > > +static const struct kobj_attribute fw_cfg_rev_attr = { > > .attr = { .name = "rev", .mode = S_IRUSR }, > > .show = fw_cfg_showrev, > > }; > > > > base-commit: 92bf22614b21a2706f4993b278017e437f7785b3 > > -- > > 2.30.1 > >
On Thu, Feb 11, 2021 at 9:41 PM 'Sami Tolvanen' via Clang Built Linux <clang-built-linux@googlegroups.com> wrote: > > Hi Nathan, > > On Thu, Feb 11, 2021 at 11:43 AM Nathan Chancellor <nathan@kernel.org> wrote: > > > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > > which violates clang's CFI checking because fw_cfg_showrev()'s second > > parameter is 'struct attribute', whereas the ->show() member of 'struct > > kobj_structure' expects the second parameter to be of type 'struct > > kobj_attribute'. > > > > $ cat /sys/firmware/qemu_fw_cfg/rev > > 3 > > > > $ dmesg | grep "CFI failure" > > [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8): > > > > Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where > > this would have been caught automatically by the incompatible pointer > > types compiler warning. Update fw_cfg_showrev() accordingly. > > > > Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") > > Link: https://github.com/ClangBuiltLinux/linux/issues/1299 > > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > > Looks good to me. Thank you for sending the patch! > > Reviewed-by: Sami Tolvanen <samitolvanen@google.com> > Environment: Linux v5.11-10201-gc03c21ba6f4e plus Clang-CFI as of 24-Feb-2021 on top built with LLVM v13-git. Tested-by: Sedat Dilek <sedat.dilek@gmail.com> - Sedat -
On Thu, Feb 11, 2021 at 12:42:58PM -0700, Nathan Chancellor wrote: > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > which violates clang's CFI checking because fw_cfg_showrev()'s second > parameter is 'struct attribute', whereas the ->show() member of 'struct > kobj_structure' expects the second parameter to be of type 'struct > kobj_attribute'. > > $ cat /sys/firmware/qemu_fw_cfg/rev > 3 > > $ dmesg | grep "CFI failure" > [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8): > > Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where > this would have been caught automatically by the incompatible pointer > types compiler warning. Update fw_cfg_showrev() accordingly. > > Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") > Link: https://github.com/ClangBuiltLinux/linux/issues/1299 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> Ah, nice, yes. Reviewed-by: Kees Cook <keescook@chromium.org> Michael, are you able to take this? I can snag it if needed. -Kees > --- > drivers/firmware/qemu_fw_cfg.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c > index 0078260fbabe..172c751a4f6c 100644 > --- a/drivers/firmware/qemu_fw_cfg.c > +++ b/drivers/firmware/qemu_fw_cfg.c > @@ -299,15 +299,13 @@ static int fw_cfg_do_platform_probe(struct platform_device *pdev) > return 0; > } > > -static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char *buf) > +static ssize_t fw_cfg_showrev(struct kobject *k, struct kobj_attribute *a, > + char *buf) > { > return sprintf(buf, "%u\n", fw_cfg_rev); > } > > -static const struct { > - struct attribute attr; > - ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf); > -} fw_cfg_rev_attr = { > +static const struct kobj_attribute fw_cfg_rev_attr = { > .attr = { .name = "rev", .mode = S_IRUSR }, > .show = fw_cfg_showrev, > }; > > base-commit: 92bf22614b21a2706f4993b278017e437f7785b3 > -- > 2.30.1 >
On 2/11/21 8:42 PM, Nathan Chancellor wrote: > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > which violates clang's CFI checking because fw_cfg_showrev()'s second > parameter is 'struct attribute', whereas the ->show() member of 'struct > kobj_structure' expects the second parameter to be of type 'struct > kobj_attribute'. > > $ cat /sys/firmware/qemu_fw_cfg/rev > 3 > > $ dmesg | grep "CFI failure" > [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8): > > Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where > this would have been caught automatically by the incompatible pointer > types compiler warning. Update fw_cfg_showrev() accordingly. > > Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") > Link: https://github.com/ClangBuiltLinux/linux/issues/1299 > Signed-off-by: Nathan Chancellor <nathan@kernel.org> > --- > drivers/firmware/qemu_fw_cfg.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Thu, 11 Feb 2021 12:42:58 -0700, Nathan Chancellor wrote: > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > which violates clang's CFI checking because fw_cfg_showrev()'s second > parameter is 'struct attribute', whereas the ->show() member of 'struct > kobj_structure' expects the second parameter to be of type 'struct > kobj_attribute'. > > $ cat /sys/firmware/qemu_fw_cfg/rev > 3 > > [...] Applied to kspp/cfi/cleanups, thanks! [1/1] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute https://git.kernel.org/kees/c/f5c4679d6c49
On Thu, Feb 25, 2021 at 10:25 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, 11 Feb 2021 12:42:58 -0700, Nathan Chancellor wrote: > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > > which violates clang's CFI checking because fw_cfg_showrev()'s second > > parameter is 'struct attribute', whereas the ->show() member of 'struct > > kobj_structure' expects the second parameter to be of type 'struct > > kobj_attribute'. > > > > $ cat /sys/firmware/qemu_fw_cfg/rev > > 3 > > > > [...] > > Applied to kspp/cfi/cleanups, thanks! > > [1/1] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute > https://git.kernel.org/kees/c/f5c4679d6c49 > I have queued this up in my custom patchset (for-5.12/kspp-cfi-cleanups-20210225). What is the plan to get this upstream? Feel free to add my: Tested-by: Sedat Dilek <sedat.dilek@gmail.com> - Sedat -
On Fri, Apr 02, 2021 at 08:42:07AM +0200, Sedat Dilek wrote: > On Thu, Feb 25, 2021 at 10:25 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Thu, 11 Feb 2021 12:42:58 -0700, Nathan Chancellor wrote: > > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > > > which violates clang's CFI checking because fw_cfg_showrev()'s second > > > parameter is 'struct attribute', whereas the ->show() member of 'struct > > > kobj_structure' expects the second parameter to be of type 'struct > > > kobj_attribute'. > > > > > > $ cat /sys/firmware/qemu_fw_cfg/rev > > > 3 > > > > > > [...] > > > > Applied to kspp/cfi/cleanups, thanks! > > > > [1/1] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute > > https://git.kernel.org/kees/c/f5c4679d6c49 > > > > I have queued this up in my custom patchset > (for-5.12/kspp-cfi-cleanups-20210225). > > What is the plan to get this upstream? I haven't sent it to Linus yet -- I was expecting to batch more of these and send them for v5.13. (But if the kvm folks snag it, that's good too.) -Kees > > Feel free to add my: > > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> > > - Sedat -
On Fri, Apr 02, 2021 at 11:25:42AM -0700, Kees Cook wrote: > On Fri, Apr 02, 2021 at 08:42:07AM +0200, Sedat Dilek wrote: > > On Thu, Feb 25, 2021 at 10:25 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > On Thu, 11 Feb 2021 12:42:58 -0700, Nathan Chancellor wrote: > > > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > > > > which violates clang's CFI checking because fw_cfg_showrev()'s second > > > > parameter is 'struct attribute', whereas the ->show() member of 'struct > > > > kobj_structure' expects the second parameter to be of type 'struct > > > > kobj_attribute'. > > > > > > > > $ cat /sys/firmware/qemu_fw_cfg/rev > > > > 3 > > > > > > > > [...] > > > > > > Applied to kspp/cfi/cleanups, thanks! > > > > > > [1/1] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute > > > https://git.kernel.org/kees/c/f5c4679d6c49 > > > > > > > I have queued this up in my custom patchset > > (for-5.12/kspp-cfi-cleanups-20210225). > > > > What is the plan to get this upstream? > > I haven't sent it to Linus yet -- I was expecting to batch more of these > and send them for v5.13. (But if the kvm folks snag it, that's good > too.) I am going to be putting the CFI series through its paces on both arm64 and x86_64 over the next week or so on several different machines (in fact, I am writing up a report right now) so I will probably have some more of these as I find them. Cheers, Nathan
On Fri, Apr 2, 2021 at 8:31 PM Nathan Chancellor <nathan@kernel.org> wrote: > > On Fri, Apr 02, 2021 at 11:25:42AM -0700, Kees Cook wrote: > > On Fri, Apr 02, 2021 at 08:42:07AM +0200, Sedat Dilek wrote: > > > On Thu, Feb 25, 2021 at 10:25 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > > > On Thu, 11 Feb 2021 12:42:58 -0700, Nathan Chancellor wrote: > > > > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > > > > > which violates clang's CFI checking because fw_cfg_showrev()'s second > > > > > parameter is 'struct attribute', whereas the ->show() member of 'struct > > > > > kobj_structure' expects the second parameter to be of type 'struct > > > > > kobj_attribute'. > > > > > > > > > > $ cat /sys/firmware/qemu_fw_cfg/rev > > > > > 3 > > > > > > > > > > [...] > > > > > > > > Applied to kspp/cfi/cleanups, thanks! > > > > > > > > [1/1] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute > > > > https://git.kernel.org/kees/c/f5c4679d6c49 > > > > > > > > > > I have queued this up in my custom patchset > > > (for-5.12/kspp-cfi-cleanups-20210225). > > > > > > What is the plan to get this upstream? > > > > I haven't sent it to Linus yet -- I was expecting to batch more of these > > and send them for v5.13. (But if the kvm folks snag it, that's good > > too.) > > I am going to be putting the CFI series through its paces on both arm64 > and x86_64 over the next week or so on several different machines (in > fact, I am writing up a report right now) so I will probably have some > more of these as I find them. > This was just a friendly ping. Sami has sent some patches which I reported in the early stage of clang-cfi (x86-64) through subtree maintainers. It's up to you Nathan or kvm folks. Upstreamed patches means to me a RDC-ed custom patchset. - Sedat -
diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index 0078260fbabe..172c751a4f6c 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -299,15 +299,13 @@ static int fw_cfg_do_platform_probe(struct platform_device *pdev) return 0; } -static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char *buf) +static ssize_t fw_cfg_showrev(struct kobject *k, struct kobj_attribute *a, + char *buf) { return sprintf(buf, "%u\n", fw_cfg_rev); } -static const struct { - struct attribute attr; - ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf); -} fw_cfg_rev_attr = { +static const struct kobj_attribute fw_cfg_rev_attr = { .attr = { .name = "rev", .mode = S_IRUSR }, .show = fw_cfg_showrev, };
fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), which violates clang's CFI checking because fw_cfg_showrev()'s second parameter is 'struct attribute', whereas the ->show() member of 'struct kobj_structure' expects the second parameter to be of type 'struct kobj_attribute'. $ cat /sys/firmware/qemu_fw_cfg/rev 3 $ dmesg | grep "CFI failure" [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8): Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where this would have been caught automatically by the incompatible pointer types compiler warning. Update fw_cfg_showrev() accordingly. Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg device") Link: https://github.com/ClangBuiltLinux/linux/issues/1299 Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- drivers/firmware/qemu_fw_cfg.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) base-commit: 92bf22614b21a2706f4993b278017e437f7785b3