Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ot_kmac: fix cSHAKE/KMAC prefix handling #19

Conversation

loiclefort
Copy link

@loiclefort loiclefort commented Aug 18, 2023

Defer prefix processing to the point where cSHAKE init is done to avoid trying to parse prefixes in other modes. If prefix parsing fails, just emit a guest_error trace and continue processing (the resulting digest will obviously be wrong in this case).

KMAC prefix is still checked at START if KMAC_EN is set. This test is now similar to the RTL implementation, by matching the first 6 bytes of the prefix with a known value.

  • some minor non-related fixes

@loiclefort loiclefort force-pushed the ot-kmac-fix-cshake-prefix-handling branch 3 times, most recently from c174aec to d8a3bc5 Compare August 18, 2023 15:53
loiclefort and others added 3 commits August 18, 2023 17:54
Defer prefix processing to the point where cSHAKE init is done to avoid trying
to parse prefixes in other modes. If prefix parsing fails, just emit a
guest_error trace and continue processing (the resulting digest will obviously
be wrong in this case).

KMAC prefix is still checked at START if KMAC_EN is set. This test is now
similar to the RTL implementation, by matching the first 6 bytes of the prefix
with a known value.

Signed-off-by: Loïc Lefort <loic@rivosinc.com>
Signed-off-by: Loïc Lefort <loic@rivosinc.com>
Tweak code formatting to prevent clang-format from overflowing the 80 columns
limit.

Signed-off-by: Emmanuel Blot <eblot@rivosinc.com>
@loiclefort loiclefort force-pushed the ot-kmac-fix-cshake-prefix-handling branch from d8a3bc5 to 05fa8fe Compare August 18, 2023 15:54
Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@loiclefort loiclefort merged commit 1cd746b into lowRISC:ot-earlgrey-8.0.2 Aug 18, 2023
@loiclefort loiclefort deleted the ot-kmac-fix-cshake-prefix-handling branch August 18, 2023 15:56
loiclefort pushed a commit that referenced this pull request Dec 5, 2023
virtio_load() as a whole should run in coroutine context because it
reads from the migration stream and we don't want this to block.

However, it calls virtio_set_features_nocheck() and devices don't
expect their .set_features callback to run in a coroutine and therefore
call functions that may not be called in coroutine context. To fix this,
drop out of coroutine context for calling virtio_set_features_nocheck().

Without this fix, the following crash was reported:

  #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
  #1  0x00007efc738c05d3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
  #2  0x00007efc73873d26 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
  #3  0x00007efc738477f3 in __GI_abort () at abort.c:79
  #4  0x00007efc7384771b in __assert_fail_base (fmt=0x7efc739dbcb8 "", assertion=assertion@entry=0x560aebfbf5cf "!qemu_in_coroutine()",
     file=file@entry=0x560aebfcd2d4 "../block/graph-lock.c", line=line@entry=275, function=function@entry=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:92
  #5  0x00007efc7386ccc6 in __assert_fail (assertion=0x560aebfbf5cf "!qemu_in_coroutine()", file=0x560aebfcd2d4 "../block/graph-lock.c", line=275,
     function=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:101
  #6  0x0000560aebcd8dd6 in bdrv_register_buf ()
  #7  0x0000560aeb97ed97 in ram_block_added.llvm ()
  #8  0x0000560aebb8303f in ram_block_add.llvm ()
  #9  0x0000560aebb834fa in qemu_ram_alloc_internal.llvm ()
  #10 0x0000560aebb2ac98 in vfio_region_mmap ()
  #11 0x0000560aebb3ea0f in vfio_bars_register ()
  #12 0x0000560aebb3c628 in vfio_realize ()
  #13 0x0000560aeb90f0c2 in pci_qdev_realize ()
  #14 0x0000560aebc40305 in device_set_realized ()
  #15 0x0000560aebc48e07 in property_set_bool.llvm ()
  #16 0x0000560aebc46582 in object_property_set ()
  #17 0x0000560aebc4cd58 in object_property_set_qobject ()
  #18 0x0000560aebc46ba7 in object_property_set_bool ()
  #19 0x0000560aeb98b3ca in qdev_device_add_from_qdict ()
  #20 0x0000560aebb1fbaf in virtio_net_set_features ()
  #21 0x0000560aebb46b51 in virtio_set_features_nocheck ()
  #22 0x0000560aebb47107 in virtio_load ()
  #23 0x0000560aeb9ae7ce in vmstate_load_state ()
  #24 0x0000560aeb9d2ee9 in qemu_loadvm_state_main ()
  #25 0x0000560aeb9d45e1 in qemu_loadvm_state ()
  #26 0x0000560aeb9bc32c in process_incoming_migration_co.llvm ()
  #27 0x0000560aebeace56 in coroutine_trampoline.llvm ()

Cc: qemu-stable@nongnu.org
Buglink: https://issues.redhat.com/browse/RHEL-832
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20230905145002.46391-3-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 92e2e6a)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants