Skip to content

Conversation

@anordal
Copy link
Contributor

@anordal anordal commented Apr 17, 2024

Description

The first commit moves the initialization of pxIPHeader. This fixes a possible nullpointer dereference.

The second commit reduces the scope of pxIPHeader and other local variables. Dropped.

Test Steps

I am using this CMake hack to compile my project with clang-tidy:

# Setting *_CLANG_TIDY tells CMake to insert itself as a compiler wrapper,
# underneath Ccache, that also runs Clang-tidy. Thus, Clang-tidy is rerun
# whenever the compiler is, as determined by Ninja and Ccache. For Ccache
# invalidation to work when settings are changed, the settings must therefore be
# in the arguments, not some ".clang-tidy" file that Ccache doesn't know about.
if(CMAKE_C_COMPILER_ID MATCHES "Clang")
    set(tidy_checks
        -clang-analyzer-security.insecureAPI.*
        -clang-analyzer-deadcode.DeadStores
        # -clang-analyzer-core.NullDereference
    )
    string(JOIN "," tidy_checks_str ${tidy_checks})
    set(CMAKE_C_CLANG_TIDY
        clang-tidy
        --use-color
        --checks=${tidy_checks_str}
        --warnings-as-errors=*,${tidy_checks_str}
    )
endif()

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.
  • I have run clang-tidy on the changed files to confirm that the changes at least make clang-tidy happy.

Related Issue

#1136

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@anordal anordal requested a review from a team as a code owner April 17, 2024 14:23
Copy link
Contributor

@shubnil shubnil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating the change. This is a good catch. I have added a minor comment.

@anordal anordal force-pushed the tcp-transmission-fix-nullptrderef branch from 9894d78 to f94c8f0 Compare April 18, 2024 14:54
Fix a possible null pointer derefernce:
The pointer returned by pxDuplicateNetworkBufferWithDescriptor()
was being dereferenced outside its null checks.
This could only happen if ipconfigZERO_COPY_TX_DRIVER was enabled.

Conveniently, the statement that used the pointer could be moved
into the scope of the null check.
@shubnil shubnil force-pushed the tcp-transmission-fix-nullptrderef branch from f94c8f0 to 222f93c Compare April 19, 2024 08:40
@xuelix xuelix requested a review from shubnil April 19, 2024 17:40
@tony-josi-aws
Copy link
Member

@anordal , thanks for creating this PR

@tony-josi-aws tony-josi-aws merged commit 0a5f7fb into FreeRTOS:main Apr 22, 2024
@anordal anordal deleted the tcp-transmission-fix-nullptrderef branch April 22, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants