Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: sockets: Fix sendmsg() user mode param checks #25932

Merged

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented Jun 3, 2020

If we are calling sendmsg() for a connected socket, then msg_namelen
is 0 and msg_name is NULL. Check these allowed values properly.

Also modify unit tests so that we test this scenario.

Fixes #25925

Signed-off-by: Jukka Rissanen jukka.rissanen@linux.intel.com

@jukkar jukkar added this to the v2.3.0 milestone Jun 3, 2020
@jukkar jukkar requested review from carlescufi and ABOSTM June 3, 2020 09:43
@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Jun 3, 2020
@ABOSTM
Copy link
Collaborator

ABOSTM commented Jun 3, 2020

What about msg_control filed ? is it also optional ?
If not optional then tests/net/socket/socketpair/ should be updated to set this field, otherwise test will still be failed.
If optional the same behaviour than msg_name should be applied to msg_control .

If we are calling sendmsg() for a connected socket, then msg_namelen
is 0 and msg_name is NULL. Check these allowed values properly.

Also modify unit tests so that we test this scenario.

Fixes zephyrproject-rtos#25925

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
If we are calling sendmsg() without any aux data, then msg_controllen
is 0 and msg_control is NULL. Check these allowed values properly.

Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>
@jukkar jukkar force-pushed the issue-25925-sendmsg-param-checks branch from a07c531 to 5e2eedb Compare June 3, 2020 10:07
@jukkar
Copy link
Member Author

jukkar commented Jun 3, 2020

What about msg_control filed ? is it also optional ?

Yes, it is optional, just forgot to add the checks.

@zephyrbot
Copy link
Collaborator

All checks passed.

checkpatch (informational only, not a failure)

-:120: WARNING:LONG_LINE: line over 80 characters
#120: FILE: tests/net/socket/udp/src/main.c:972:
+			 ztest_user_unit_test(test_v4_sendmsg_recvfrom_no_aux_data),

- total: 0 errors, 1 warnings, 110 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPDX_LICENSE_TAG SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

Copy link
Collaborator

@ABOSTM ABOSTM left a comment

Choose a reason for hiding this comment

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

LGTM

@carlescufi carlescufi merged commit a2f0a9e into zephyrproject-rtos:master Jun 3, 2020
@jukkar jukkar deleted the issue-25925-sendmsg-param-checks branch June 3, 2020 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: net: socket: socketpair: fails due to empty message header name
4 participants