From 3e98b2dfe421ed97cf98017c64be0a74cfb10168 Mon Sep 17 00:00:00 2001 From: iceman1001 Date: Mon, 14 Sep 2026 15:05:31 +0200 Subject: [PATCH] hf legic: stop eload losing everything after its first packet 'hf legic eload' writes emulator memory as a burst of back-to-back packets. CMD_HF_LEGIC_ESET called FpgaDownloadAndGo(FPGA_BITSTREAM_HF) on every one of them, and when that actually had a bitstream to download the device stopped servicing USB for long enough that the packets already in flight behind it were dropped. Reproducible, and it only shows after a command that left a different bitstream loaded, which is why it has gone unnoticed: hf iclass eload ... # loads FPGA_BITSTREAM_HF_15 hf legic eload -f x.bin # first packet downloads FPGA_BITSTREAM_HF hf legic esave --1024 # 404 bytes of 1024 come back as zeros The boundary is always 619, which is max_cmd_data_size minus sizeof(legic_packet_t) -- exactly one packet. Back to back a second time it works, because the bitstream is then already loaded. The 'fast push mode' in legic_seteml() is not involved: block_after_ACK only applies to OLD frames and these are NG. So the handler does no FPGA work at all now. The comment it used to carry -- 'if it is called later, it might destroy the Emulator Memory' -- was aiming at a real problem but solving it at the wrong end. init_tag() in legicrfsim.c is where the bitstream is genuinely needed, and it was using the plain variant twelve lines above 'legic_mem = BigBuf_get_EM_addr()', so 'hf legic sim' was wiping the image it was about to serve. That one becomes _keep_EM. Five sibling handlers carry the same copy-pasted comment and the same wipe-before-use pattern and are switched to _keep_EM as well: CMD_LF_EM4X50_SIM (em4x50_sim reads its tag out of emulator memory), CMD_LF_EM4X50_ESET, CMD_HF_ISO15693_EML_SETMEM, EML_GETMEM and CMD_HF_MIFARE_EML_MEMCLR. Only the LEGIC path has a reproducer; the rest is the same fix applied where the same mistake is visible. Verified on an RDV4: the reproducer above now loses 0 bytes over three runs, 'hf legic sim' reports the MCD/MSN of the uploaded image, and eload/esave still round-trips byte-identical for MIFARE Classic 4K, iso15693 and iCLASS. Co-Authored-By: Claude Opus 5 (1M context) --- armsrc/appmain.c | 56 ++++++++++++++++++++++++++------------------- armsrc/legicrfsim.c | 6 +++-- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/armsrc/appmain.c b/armsrc/appmain.c index 18279feb2..b87b5e9c2 100644 --- a/armsrc/appmain.c +++ b/armsrc/appmain.c @@ -1827,11 +1827,13 @@ static void PacketReceived(PacketCommandNG *packet) { } case CMD_LF_EM4X50_SIM: { //----------------------------------------------------------------------------- - // Note: we call FpgaDownloadAndGo(FPGA_BITSTREAM_LF) here although FPGA is not - // involved in dealing with emulator memory. But if it is called later, it might - // destroy the Emulator Memory. + // The FPGA is not involved in dealing with emulator memory, but loading a + // bitstream later would wipe it, so the load is brought forward to here. + // It has to be the _keep_EM variant: FpgaDownloadAndGo() frees and clears + // the whole of BigBuf to make room to decompress, emulator memory included, + // which is the very thing this is here to preserve. //----------------------------------------------------------------------------- - FpgaDownloadAndGo(FPGA_BITSTREAM_LF); + FpgaDownloadAndGo_keep_EM(FPGA_BITSTREAM_LF); em4x50_sim((const uint32_t *)packet->data.asBytes, true); break; } @@ -1841,11 +1843,13 @@ static void PacketReceived(PacketCommandNG *packet) { } case CMD_LF_EM4X50_ESET: { //----------------------------------------------------------------------------- - // Note: we call FpgaDownloadAndGo(FPGA_BITSTREAM_LF) here although FPGA is not - // involved in dealing with emulator memory. But if it is called later, it might - // destroy the Emulator Memory. + // The FPGA is not involved in dealing with emulator memory, but loading a + // bitstream later would wipe it, so the load is brought forward to here. + // It has to be the _keep_EM variant: FpgaDownloadAndGo() frees and clears + // the whole of BigBuf to make room to decompress, emulator memory included, + // which is the very thing this is here to preserve. //----------------------------------------------------------------------------- - FpgaDownloadAndGo(FPGA_BITSTREAM_LF); + FpgaDownloadAndGo_keep_EM(FPGA_BITSTREAM_LF); if (packet->length < sizeof(em4x50_eset_t)) { reply_ng(CMD_LF_EM4X50_ESET, PM3_EINVARG, NULL, 0); @@ -1958,11 +1962,13 @@ static void PacketReceived(PacketCommandNG *packet) { } case CMD_HF_ISO15693_EML_SETMEM: { //----------------------------------------------------------------------------- - // Note: we call FpgaDownloadAndGo(FPGA_BITSTREAM_HF_15) here although FPGA is not - // involved in dealing with emulator memory. But if it is called later, it might - // destroy the Emulator Memory. + // The FPGA is not involved in dealing with emulator memory, but loading a + // bitstream later would wipe it, so the load is brought forward to here. + // It has to be the _keep_EM variant: FpgaDownloadAndGo() frees and clears + // the whole of BigBuf to make room to decompress, emulator memory included, + // which is the very thing this is here to preserve. //----------------------------------------------------------------------------- - FpgaDownloadAndGo(FPGA_BITSTREAM_HF_15); + FpgaDownloadAndGo_keep_EM(FPGA_BITSTREAM_HF_15); struct p { uint32_t offset; uint16_t count; @@ -1973,7 +1979,8 @@ static void PacketReceived(PacketCommandNG *packet) { break; } case CMD_HF_ISO15693_EML_GETMEM: { - FpgaDownloadAndGo(FPGA_BITSTREAM_HF_15); + // keep the Emulator Memory, we are about to read it back + FpgaDownloadAndGo_keep_EM(FPGA_BITSTREAM_HF_15); struct p { uint32_t offset; uint16_t length; @@ -2129,12 +2136,14 @@ static void PacketReceived(PacketCommandNG *packet) { break; } case CMD_HF_LEGIC_ESET: { - //----------------------------------------------------------------------------- - // Note: we call FpgaDownloadAndGo(FPGA_BITSTREAM_HF) here although FPGA is not - // involved in dealing with emulator memory. But if it is called later, it might - // destroy the Emulator Memory. - //----------------------------------------------------------------------------- - FpgaDownloadAndGo(FPGA_BITSTREAM_HF); + // No FPGA work here on purpose. An upload arrives as a burst of + // back-to-back packets, and a bitstream download in the first one takes + // long enough that the device stops servicing USB and the packets behind + // it are lost -- `hf legic eload` after a command that left a different + // bitstream loaded used to write only its first 619 bytes. + // init_tag() in legicrfsim.c loads the bitstream when the simulation + // actually starts, and does it with the _keep_EM variant so the content + // uploaded here survives. legic_packet_t *payload = (legic_packet_t *) packet->data.asBytes; emlSet(payload->data, payload->offset, payload->len); break; @@ -2540,11 +2549,12 @@ static void PacketReceived(PacketCommandNG *packet) { //----------------------------------------------------------------------------- // Work with emulator memory // - // Note: we call FpgaDownloadAndGo(FPGA_BITSTREAM_HF) here although FPGA is not - // involved in dealing with emulator memory. But if it is called later, it might - // destroy the Emulator Memory. + // The FPGA is not involved here, but loading a bitstream later would wipe + // emulator memory, so the load is brought forward. _keep_EM because + // FpgaDownloadAndGo() frees and clears all of BigBuf to decompress into, + // which would drop the allocation emlClearMem() is about to fill in. //----------------------------------------------------------------------------- - FpgaDownloadAndGo(FPGA_BITSTREAM_HF); + FpgaDownloadAndGo_keep_EM(FPGA_BITSTREAM_HF); // Not only clears the emulator memory, // also sets default MIFARE values for sector trailers. diff --git a/armsrc/legicrfsim.c b/armsrc/legicrfsim.c index f66d2e3b6..923f35f7d 100644 --- a/armsrc/legicrfsim.c +++ b/armsrc/legicrfsim.c @@ -311,8 +311,10 @@ static int32_t init_card(uint8_t cardtype, legic_card_select_t *p_card) { } static void init_tag(void) { - // configure FPGA - FpgaDownloadAndGo(FPGA_BITSTREAM_HF); + // configure FPGA. _keep_EM because legic_mem below is emulator memory, which + // is where `hf legic eload` put the tag content -- the plain variant frees and + // clears the whole of BigBuf to decompress into and would wipe it + FpgaDownloadAndGo_keep_EM(FPGA_BITSTREAM_HF); FpgaWriteConfWord(FPGA_MAJOR_MODE_HF_SIMULATOR | FPGA_HF_SIMULATOR_MODULATE_212K); SetAdcMuxFor(ADC_MUXSEL_HIPKD);