flipperdevices / flipperzero-good-faps

Flipper Zero Official Apps maintained by Flipper Team and Friends
GNU General Public License v3.0
285 stars 54 forks source link

Update MFKey to v2.7 #231

Closed noproto closed 2 months ago

noproto commented 3 months ago

What's new

Verification

Here are several test Mfkey32 nonces you can store in .mfkey32.log:

Sector 0 key A cuid 2a234f80 nt0 be771bb5 nr0 2328dd4a ar0 caefa8a9 nt1 2be39d62 nr1 27c7fe47 ar1 3c270451
Sector 3 key A cuid 2a234f80 nt0 97dab47b nr0 b4096aba ar0 49970dc0 nt1 f9e89fba nr1 24d2d128 ar1 30af6a2d
Sector 1 key A cuid 2a234f80 nt0 1fd5536e nr0 5612a105 ar0 923af95b nt1 60b8b7b6 nr1 f5e02214 ar1 fcb5727f
Sec 4 key A cuid 2a234f80 nt0 9f7f775e nr0 ef5329a2 ar0 52f12523 nt1 c9bdecca nr1 2c3a03f2 ar1 ef7aae07
Sec 8 key A cuid 2a234f80 nt0 616e46a1 nr0 e53acc1d ar0 ebd10c77 nt1 2102fcc8 nr1 d3c09dcb ar1 2aebc1b1
Sec 0 key A cuid 70132f56 nt0 9b6b5285 nr0 75f3ea90 ar0 36f5fc03 nt1 0260020d nr1 ac65339b ar1 52831223
Sec 0 key A cuid 2f82add7 nt0 db38301a nr0 680a07c4 ar0 0d36d574 nt1 484ba9d0 nr1 a0693d69 ar1 c37c0f79
Sector 0 key A cuid 00000000 nt0 8092e400 nr0 095e79d2 ar0 5325c560 nt1 997bd0e9 nr1 6bfbcfc2 ar1 642a2b68
Sector 0 key A cuid 00000001 nt0 22a02fc1 nr0 388a1c25 ar0 3fad2d95 nt1 e7ff1dd0 nr1 6bb02d3b ar1 4be867ba
Sector 2 key A cuid 00000000 nt0 b35ceca2 nr0 6cc65eac ar0 e3d3f37f nt1 8ce6ca54 nr1 2610fefa ar1 a32656ca
Sector 2 key A cuid 00000000 nt0 13c015a6 nr0 1e29d6d1 ar0 67d0a3f8 nt1 2c94031f nr1 7b4e2a4f ar1 65f9d4e0
Sector 2 key A cuid 00000000 nt0 10bb6eb7 nr0 03a907d2 ar0 19d0dccc nt1 2c0dabf7 nr1 6982602c ar1 02512240

Checklist (For Reviewer)

skotopes commented 2 months ago

@noproto super sorry for delay, I've missed email with notification.

Also would you like to be maintainer in this repository or would you like to have mfkey in your repo(like betsy did with picopass)?

PS: also don't hesitate to ping us if there are any delays.

skotopes commented 2 months ago

I see crash, looks like missing furi_thread_join. I think I've pressed either ok or back on complete screen.

#0  0x080123a0 in __furi_crash_implementation () at furi/core/check.c:170
#1  0x08015c82 in furi_thread_free (thread=0x2000cc40) at furi/core/thread.c:205
#2  0x20019240 in mfkey_main () at /Users/aku/Work/Flipper/flipperzero-good-faps/mfkey/mfkey.c:896
#3  0x0805d086 in flipper_application_thread (context=0x2000bd58) at lib/flipper_application/flipper_application.c:239
#4  0x08015b8e in furi_thread_body (context=0x2000c8d8) at furi/core/thread.c:103
#5  0x08015b3e in furi_thread_catch () at furi/core/thread.c:75
(gdb) p *thread
$2 = {
  container = {
    pxDummy1 = 0x2000e27c,
    xDummy3 = {{
        xDummy2 = 0,
        pvDummy3 = {0x20000ff8 <xTasksWaitingTermination+8>, 0x20000ff8 <xTasksWaitingTermination+8>, 0x2000cc40, 0x20000ff0 <xTasksWaitingTermination>}
      }, {
        xDummy2 = 16,
        pvDummy3 = {0x2000cb6c, 0x2000cb6c, 0x2000cc40, 0x0}
      }},
    uxDummy5 = 16,
    pxDummy6 = 0x2000dae8,
    ucDummy7 = "MFKey Worker", '\000' <repeats 19 times>,
    pxDummy8 = 0x2000e2e0,
    uxDummy10 = {47, 0},
    uxDummy12 = {16, 0},
    pvDummy15 = {0x2000cc40},
    ulDummy16 = 3538578007,
    ulDummy18 = {0, 0, 0},
    ucDummy19 = "\000\000"
  },
  stack_buffer = 0x2000dae8,
  state = FuriThreadStateStopped,
  ret = 0,
  callback = 0x200190f1 <mfkey_worker_thread>,
  context = 0x2000caa0,
  state_callback = 0x0,
  state_context = 0x0,
  signal_callback = 0x0,
  signal_context = 0x0,
  name = 0x2000bcd8 "MFKey Worker",
  appid = 0x2000bcc8 "mfkey",
  priority = FuriThreadPriorityNone,
  stack_size = 2048,
  heap_size = 0,
  output = {
    write_callback = 0x0,
    buffer = 0x2000bcb0
  },
  is_service = false,
  heap_trace_enabled = false,
  is_active = true
}
noproto commented 2 months ago

So unfortunately I'm not able to reproduce a crash - I attempted for 4 hours with various tests - or locate the condition in MFKey which would result in that behavior. It'll be very helpful if I can get steps to reproduce it so I can get the bug closed out. Reviewing the diff of the changes on this PR, a crash in 2.7 should be applicable to MFKey 2.6 and earlier versions if it is related to the thread joining.

Also I'd be happy to be a maintainer for this repo to keep MFKey up to date. The recent MFKey changes are to support the firmware Nested implementation that is halfway complete.

skotopes commented 2 months ago

@noproto once again sorry for delay with this PR.

I also was not able to reproduce crash anymore. I was able to reproduce Timer thread lockup caused by event accumulation, but that is kinda expected: worker doesn't give CPU time to lower priority threads.

As an apologies for delay I've cleaned up code a little bit.