Skip to content

Commit

Permalink
Fix ATS handling in tags that NAK RATS
Browse files Browse the repository at this point in the history
pcd_14a_reader_ats_request didn't check for NAK so in case the tag NAKd
the RATS it would return a zero length ATS which would cause an
underflow in pcd_14a_reader_scan_once, that in turn resulted in
HF_ERR_ATS being returned due to an invalid ATS length.
  • Loading branch information
augustozanellato committed Oct 17, 2023
1 parent 2beb8f7 commit cb18784
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file.
This project uses the changelog in accordance with [keepchangelog](http://keepachangelog.com/). Please use this to write notable changes, which is not the same as git commit log...

## [unreleased][unreleased]
- Fixed ATS handling in tags that NAK RATS (@augustozanellato)
- Changed battery level curves based on experimental measures (@spp2000)
- Added multithreading on Nested and StaticNested (@xianglin1998)
- Fixed factory reset hanging (@augustozanellato)
Expand Down
32 changes: 19 additions & 13 deletions firmware/application/src/rfid/reader/hf/rc522.c
Original file line number Diff line number Diff line change
Expand Up @@ -645,20 +645,23 @@ uint8_t pcd_14a_reader_scan_once(picc_14a_tag_t *tag) {
// Tag supports 14443-4, sending RATS
uint16_t ats_size;
status = pcd_14a_reader_ats_request(tag->ats, &ats_size, 0xFF * 8);
ats_size -= 2; // size returned by pcd_14a_reader_ats_request includes CRC
if (ats_size > 254) {
NRF_LOG_INFO("Invalid ATS > 254!");
return HF_ERR_ATS;
}
tag->ats_len = ats_size;
// We do not validate ATS here as we want to report ATS as it is without breaking 14a scan
if (tag->ats[0] != ats_size - 1) {
NRF_LOG_INFO("Invalid ATS! First byte doesn't match received length");
// return HF_ERR_ATS;
}
NRF_LOG_INFO("ats status %d, length %d", status, ats_size);
if (status != HF_TAG_OK) {
NRF_LOG_INFO("Tag SAK claimed to support ATS but tag NAKd RATS");
tag->ats_len = 0;
// return HF_ERR_ATS;
} else {
ats_size -= 2; // size returned by pcd_14a_reader_ats_request includes CRC
if (ats_size > 254) {
NRF_LOG_INFO("Invalid ATS > 254!");
return HF_ERR_ATS;
}
tag->ats_len = ats_size;
// We do not validate ATS here as we want to report ATS as it is without breaking 14a scan
if (tag->ats[0] != ats_size - 1) {
NRF_LOG_INFO("Invalid ATS! First byte doesn't match received length");
// return HF_ERR_ATS;
}
}
/*
* FIXME: If there is an issue here, it will cause the label to lose its selected state.
Expand Down Expand Up @@ -706,11 +709,14 @@ uint8_t pcd_14a_reader_ats_request(uint8_t *pAts, uint16_t *szAts, uint16_t szAt

if (status != HF_TAG_OK) {
*szAts = 0;
NRF_LOG_INFO("Err at ats receive.\n");
NRF_LOG_ERROR("ATS rx error: %d", status);
return status;
} else if (*szAts == 7 && pAts[0] == 0x4) { // tag replied with NAK
*szAts = 0;
return HF_ERR_ATS;
}

// NRF_LOG_INFO("Length: %d\n", *szAts);
NRF_LOG_INFO("Received ATS length: %d\n", *szAts);

if (*szAts > 0) { *szAts = *szAts / 8; }
return HF_TAG_OK;
Expand Down

0 comments on commit cb18784

Please sign in to comment.