lwip: fix socket behaviour #5684

juhaylinen · 2017-12-11T14:04:19Z

Return NSAPI_ERROR_PARAMETER when:
Binding to a non-local address
Socket listen() is called without calling bind() first
Socket accept() is called without calling listen() first

kjbracey

Concept seems fine, but the address match needs to be a bit more general.

kjbracey · 2017-12-11T14:26:49Z

features/FEATURE_LWIP/lwip-interface/lwip_stack.c

@@ -992,6 +992,32 @@ static nsapi_error_t mbed_lwip_socket_bind(nsapi_stack_t *stack, nsapi_socket_t
        return NSAPI_ERROR_PARAMETER;
    }

+#if LWIP_IPV6
+    if (IP_IS_V6(ip_addr) && !ip6_addr_isany(&ip_addr)) {
+        const ip_addr_t *local_addr = mbed_lwip_get_ipv6_addr(&lwip_netif);


This isn't really sufficient - you need to check all addresses for all interfaces. At present this code isn't multiple-interface, but #5558 will make it so. Basically accessing the global lwip_netif from this socket call is a cheat - that won't be possible from #5558. Socket calls are not interface-specific.

And even before #5558, an interface may have multiple IPv6 addresses - there currently isn't a public API to enumerate them, so it's not obvious how an application would have found any other address than mbed_lwip_get_ipv6_addr returns - but there may be in future.

So loop over the netif_list, then over each netif's addresses, looking for a match.

You may be able collapse the code a bit with some use of generic lwIP functions - the outer if could be if !(ip_addr_isany()), then inside that loop over interfaces, then inside that loop the V4 and V6 checks.

0xc0170 · 2017-12-15T15:35:39Z

@kjbracey-arm happy with the updated commits?

/morph build

mbed-ci · 2017-12-15T15:58:13Z

Build : FAILURE

Build number : 708
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5684/

kjbracey · 2017-12-18T08:08:56Z

features/FEATURE_LWIP/lwip-interface/lwip_stack.c

@@ -992,6 +1023,10 @@ static nsapi_error_t mbed_lwip_socket_bind(nsapi_stack_t *stack, nsapi_socket_t
        return NSAPI_ERROR_PARAMETER;
    }

+    if (!ip_addr_isany(&ip_addr) && !mbed_lwip_is_local_addr(&ip_addr)) {
+        return NSAPI_ERROR_PARAMETER;


I wonder if the return error here should be "NO_ADDRESS"? Guess this is fine though.

I see Nanostack has "PARAMETER" already, so this matches.

0xc0170 · 2017-12-18T08:09:59Z

/morph build

mbed-ci · 2017-12-18T08:40:35Z

Build : FAILURE

Build number : 710
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5684/

0xc0170 · 2017-12-18T13:09:42Z

Please review the jenkins CI error Error: ip6_addr.h@140,65: ‘ip_addr_t {aka const struct ip_addr}’ has no member named ‘addr’; did you mean ‘u_addr’? Is it related?

kjbracey · 2017-12-18T13:18:41Z

You need ip_2_ip4(ip_addr) and ip_2_ip6(ip_addr) in the comparison - when doing this sort of code, make sure you check with IPv4-only, IPv6-only and IPv4+IPv6, otherwise it's easy to miss the necessary adaptation bits like that.

SeppoTakalo · 2017-12-19T10:20:34Z

@0xc0170 Please rerun the tests

0xc0170 · 2017-12-19T10:37:33Z

/morph build

mbed-ci · 2017-12-19T11:07:32Z

Build : SUCCESS

Build number : 720
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5684/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2017-12-19T11:57:06Z

Exporter Build : SUCCESS

Build number : 372
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5684/

0xc0170 · 2017-12-19T12:16:09Z

Waiting for travis fix to land, then will restart it here (plus morph tests )

mbed-ci · 2017-12-19T12:19:23Z

Test : FAILURE

Build number : 550
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5684/550

0xc0170 · 2017-12-22T11:11:33Z

@juhaylinen Can you please rebase to fix travis in this PR? we will trigger CI once done

Return NSAPI_ERROR_PARAMETER when: Binding to a non-local address Socket listen() is called without calling bind() first Socket accept() is called without calling listen() first

Update the code to check all addresses for all interfaces. Move the code from mbed_lwip_socket_bind() to a new function called mbed_lwip_is_local_addr()

juhaylinen · 2017-12-22T11:27:49Z

@0xc0170 Rebase done

0xc0170 · 2017-12-22T13:28:30Z

/morph build

mbed-ci · 2017-12-22T13:46:55Z

Build : SUCCESS

Build number : 762
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5684/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2017-12-22T16:38:09Z

Exporter Build : SUCCESS

Build number : 415
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5684/

mbed-ci · 2017-12-28T06:29:07Z

Test : SUCCESS

Build number : 593
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5684/593

0xc0170 requested review from kjbracey and mikaleppanen December 11, 2017 14:13

kjbracey reviewed Dec 11, 2017

View reviewed changes

0xc0170 added the needs: work label Dec 12, 2017

juhaylinen force-pushed the lwip-socket-fix branch from 25ce09e to 0b778b2 Compare December 12, 2017 13:32

mikaleppanen approved these changes Dec 14, 2017

View reviewed changes

0xc0170 added needs: review and removed needs: work labels Dec 15, 2017

kjbracey approved these changes Dec 18, 2017

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Dec 18, 2017

SeppoTakalo approved these changes Dec 19, 2017

View reviewed changes

0xc0170 added needs: preceding PR and removed needs: work labels Dec 20, 2017

juhaylinen added 3 commits December 22, 2017 13:21

lwip: fix socket behaviour

6d3e417

Return NSAPI_ERROR_PARAMETER when: Binding to a non-local address Socket listen() is called without calling bind() first Socket accept() is called without calling listen() first

Add function to check if address is local

52353d2

Update the code to check all addresses for all interfaces. Move the code from mbed_lwip_socket_bind() to a new function called mbed_lwip_is_local_addr()

Fix build for IPv4+IPv6 config

32e64e7

juhaylinen force-pushed the lwip-socket-fix branch from cc53590 to 32e64e7 Compare December 22, 2017 11:24

0xc0170 added needs: CI and removed needs: preceding PR labels Dec 22, 2017

cmonr removed the needs: CI label Dec 28, 2017

cmonr merged commit 5a19f6d into ARMmbed:master Dec 28, 2017

juhaylinen deleted the lwip-socket-fix branch January 2, 2018 07:13

lwip: fix socket behaviour #5684

lwip: fix socket behaviour #5684

Uh oh!

Conversation

juhaylinen commented Dec 11, 2017

Uh oh!

kjbracey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Dec 15, 2017

Uh oh!

mbed-ci commented Dec 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Dec 18, 2017

Uh oh!

mbed-ci commented Dec 18, 2017

Uh oh!

0xc0170 commented Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kjbracey commented Dec 18, 2017

Uh oh!

SeppoTakalo commented Dec 19, 2017

Uh oh!

0xc0170 commented Dec 19, 2017

Uh oh!

mbed-ci commented Dec 19, 2017

Triggering tests

Uh oh!

mbed-ci commented Dec 19, 2017

Uh oh!

0xc0170 commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbed-ci commented Dec 19, 2017

Uh oh!

0xc0170 commented Dec 22, 2017

Uh oh!

juhaylinen commented Dec 22, 2017

Uh oh!

0xc0170 commented Dec 22, 2017

Uh oh!

mbed-ci commented Dec 22, 2017

Triggering tests

Uh oh!

mbed-ci commented Dec 22, 2017

Uh oh!

mbed-ci commented Dec 28, 2017

Uh oh!

Uh oh!

0xc0170 commented Dec 18, 2017 •

edited

Loading

0xc0170 commented Dec 19, 2017 •

edited

Loading