Bug in dwt_getframelength()?

[OSHW-Rico]

Hello,

Maybe someone is using the library and can validate my findings. Any help is appriciated.

Values stored in CPU Register R4 are invalid after calling dwt_getframelength(). If such a basic function has bugs I am really insecure about using it, but maybe it is me.

My findings show that R4 is pushed onto the stack when calling dwt_getframelength(). Internally a uint16_t is then reserved on the stack before the framelength is read back via SPI. A uint32_t value is then stored on the address of the uin16_t destroying the value of R4 stored on that stack position.

See

[br101]

Hi, unfortunately this looks like a bug in the binary only driver and I have already seen you reported it on the Qorvo forum where it belongs. I have not tried, but I think I am not seeing this bug because I'm using the interrupt and dwt_isr() and from there I get a callback where I can access the length as status->datalength...

[br101]

Carl Stehle has reported the same bug, I think, and has found that using disabling compiler optimization (-O1 or -O0) can help.

https://forum.qorvo.com/t/dw3xxx-xr6-0c-24feb2022-dw-drivers-requirements/12140/6

[OSHW-Rico]

Thanks for confirming, I will switch to callbacks. Just wanted to start slowly.

[br101]

Leaving this open, since it's likely to hit another person too, if only just to document the flaws of the binary only library of Qorvo.

[bkx1125]

It seems dwt_getframelength() is corrupting the stack. How do you disable compiler optimization (-O1 or -O0) in VSCode GUI?

[OSHW-Rico]

Disabling optimization did solve the issue. I switched to callbacks and later wrote my own library. As far as I know Qorvo will release the next version of the driver as open source again, without the need for an NDA. Maybe you can check if it is already available.

[bkx1125]

@OSHW-Rico, that is great Qorvo will release the source of the library. Do you know or expect any release date?

The second I applied a 4-byte variable for handling return of dwt_getframelenght() to ensure it didn't mess up other local variables' memory, but still it did and corrupted my function's memory. I'm not able to use ISR/callback due to a design issue. Is dwt_getframelenght() writing more than 4-byte? I'm not experienced in Zephyr memory management but I compiled it with debug options enabled. In WinOS x86 for example, arguments and local variables in debug mode are used to keep 12-byte per 4-byte integer, but in release mode of WinOS x86, it takes 4-byte per 4-byte integer. How does it work in zephyr? Or even if I reserve 4-bye for getframelength() return, the compiler optimizes it to 2-byte? I very much appreciate your thoughts.

[OSHW-Rico]

It's not a problem in RAM. It's a core register one of R1-8 which is corrupted from the library. That's why it is not helping when you reserve more memory.

Sorry i don't have a release date.

[bkx1125]

@OSHW-Rico, I created a topic for the newest driver in the Qorvo forum. Here is the link

[liamw9534]

I encountered this same bug in the most recent driver code, so as far as I can tell the issue has not been fixed by Qorvo. The ioctl() layer of functionality has pointer re-casting happening which can be extremely dangerous. The only reason for the ioctl() layer I can see is so Qorvo can compile multiple chip versions of the drivers into a single library. Practically speaking, I can only see a benefit to Qorvo (who are likely using a win32 based test tool and dev environment internally judging by some of the preprocessor defines in the code) and only downsides to most end users who must then endure obfuscation, increased code footprint and buggy driver code.

The specific problem, which took me quite a few hours to bottom out, is caused in dwt_getframelength() because a pointer of uint16_t type is required by the API compatibility layer but this is then converted to a uint32_t pointer by the DW3000 ioctl() translation layer for the DW3000 chip. When this pointer is later dereferenced, it tramples over the stack resulting in unpredictable behaviour in the calling application.

The first thing I did when I got the source code release from Qorvo was to remove the ioctl() layer and rename all the DW3000 chip driver level functions and then use an extern global dwchip structure. The code footprint is still bigger than the DW1000 driver but you do at least now have the possibility of the linker being able to optimize out unused functions, which it can't when you define function pointers all over the place.

If you are using a binary release of the Qorvo code, you can workaround the problem as follows. Below is the code used in the compatibility layer for dwt_getframelength():

uint16_t tmp = 0;
(void) rng;
dw->dwt_driver->dwt_ops->ioctl(dw, DWT_GETFRAMELENGTH, 0, (void *)&tmp);
return tmp;

You can write your own dwt_getframelength() function as follows:

uint32_t tmp = 0;
(void) rng;
dw->dwt_driver->dwt_ops->ioctl(dw, DWT_GETFRAMELENGTH, 0, (void *)&tmp);
return (uint16_t)tmp;

[br101]

Thank you for this information! Allthough I'm not sure how i could integrate it into the driver here, I'm sure it will be helpful to someone!

[br101]

That should be fixed now as we have the full source code and don't have to use the IOCTL any more