[BUG] Possible latent bug: dereference of NULL 'pxFile' in ff_file.c #60
Comments
|
@carlk3, thanks for taking the effort to report this. I think that it be will enough to add this line here : if( xFindParams.ulDirCluster == 0ul )
{
if( ( ucMode & FF_MODE_WRITE ) != 0 )
{
FF_PRINTF( "FF_Open[%s]: Path not found\n", pcPath );
}
/* The user tries to open a file but the path leading to the file does not exist. */
+ xError = FF_createERR( FF_ERR_FILE_INVALID_PATH, FF_OPEN );
}and this branch will not be taken because of Agreed? |
|
@htibosch I created a project to repro this issue and I can confirm that the fix suggested by you fixes this warning. Would you like to raise a PR for this? |
|
You are very quick, thank you for testing. |
|
@aggarg @carlk3 : could it be that the compilers see a possible problem that will not occur in reality? I tried to catch the dereference of NULL: I ran the following test: void ff_test( void )
{
#define SOURCE_1 "/ram/file_001.txt"
#define SOURCE_2 "/ram/file_002.txt"
create_file(SOURCE_1, "hello_world");
create_file(SOURCE_2, "hello_world");
ff_mkdir( "/ram/dir_01", pdFALSE );
/* Do not create "/ram/dir_02" */
#define TARGET_1 "/ram/dir_01/file_001.txt" /* Directory 'dir_01' OK. */
#define TARGET_2 "/ram/dir_02/file_002.txt" /* Directory 'dir_02' does not exist. */
ff_remove( TARGET_1 );
ff_remove( TARGET_2 );
/* Should be OK: */
int32_t rc1 = ff_rename( SOURCE_1, TARGET_1, pdTRUE );
/* Should fail: */
int32_t rc2 = ff_rename( SOURCE_2, TARGET_2, pdTRUE );
FreeRTOS_printf( ( "rename rc = %d %d\n", ( int ) rc1, ( int ) rc2 ) );
}I get this output when running the test 2 times: The thing is, when the error will be stored in the variable which I think is correct and as expected. |
|
Hi Hein,
On Mon, Jan 8, 2024 at 7:54 AM Hein Tibosch ***@***.***> wrote:
@aggarg <https://github.com/aggarg> @carlk3 <https://github.com/carlk3> :
could it be that the compilers see a possible problem that will not occur
in reality?
...
The thing is, when FF_FindDir() returns zero:
xFindParams.ulDirCluster = FF_FindDir( pxIOManager, pcPath, ( uint16_t ) xIndex, &xError );
the error will be stored in the variable xError.
I don't claim to fully understand this code, but...
That would seem to depend on these conditions in FF_FindDir:
if( ( pcToken != NULL ) &&
( ( FF_isERR( xError ) == pdFALSE ) || ( FF_GETERROR( xError )
== FF_ERR_DIR_END_OF_DIR ) ) )
{
xError = FF_createERR( FF_FINDDIR, FF_ERR_FILE_INVALID_PATH );
}
Could there be a case where, say, FF_isERR( xError ) == pdTRUE at that
point?
-Carl
|
|
Oh, never mind, in that case xError is already set.
So, maybe it is a false alarm.
…On Mon, Jan 8, 2024 at 11:14 AM Carl Kugler ***@***.***> wrote:
Hi Hein,
On Mon, Jan 8, 2024 at 7:54 AM Hein Tibosch ***@***.***>
wrote:
> @aggarg <https://github.com/aggarg> @carlk3 <https://github.com/carlk3>
> : could it be that the compilers see a possible problem that will not occur
> in reality?
>
> ...
>
> The thing is, when FF_FindDir() returns zero:
>
> xFindParams.ulDirCluster = FF_FindDir( pxIOManager, pcPath, ( uint16_t ) xIndex, &xError );
>
> the error will be stored in the variable xError.
>
I don't claim to fully understand this code, but...
That would seem to depend on these conditions in FF_FindDir:
if( ( pcToken != NULL ) &&
( ( FF_isERR( xError ) == pdFALSE ) || ( FF_GETERROR( xError
) == FF_ERR_DIR_END_OF_DIR ) ) )
{
xError = FF_createERR( FF_FINDDIR, FF_ERR_FILE_INVALID_PATH );
}
Could there be a case where, say, FF_isERR( xError ) == pdTRUE at that
point?
-Carl
|
|
A slightly different path: |
|
Did you or @aggarg find a Or did you see it happening in a (new) |
|
I haven't been able to contrive a test case that actually causes the dereference of NULL. But, I haven't spent infinite time trying. Probably, the warning can be safely ignored. |
|
Apologies for the delayed response. @htibosch I just compiled with the flag |
|
I think that we can close the issue. @carlk3 also confirmed so. |
Describe the bug
A concise description of what the bug is. If possible, that is the code is not proprietary, please upload the code in a GitHub fork such that we can reproduce the bug.
Compiling
ff_file.cwitharm-none-eabi-gcc.exe (GNU Tools for STM32 11.3.rel1.20230912-1600) 11.3.1 20220712and option
-fanalyzerI get this warning:Target
NUCLEO-L496ZG
ARMv7-M
STM32CubeIDE
Version: 1.14.0
Build: 19471_20231121_1200 (UTC)
GNU Tools for STM32 11.3.rel1.20230912-1600
Host
FreeRTOS 10.6.0
To Reproduce
Expected behavior
A concise description of what you expected to happen.
Screenshots
If applicable, add screenshots to help explain your problem.
Wireshark logs
To help us identify the issue and/or reproduce it, please attach Wireshark logs if applicable.
Additional context
Add any other context about the problem here.
e.g. code snippet to reproduce the issue.
e.g. stack trace, memory dump, debugger log, and many etc.
The text was updated successfully, but these errors were encountered: