google / packetdrill

The official Google release of packetdrill
GNU General Public License v2.0
887 stars 220 forks source link

fix access to 'sock_extended_err' when generating cmsg from expressions #68

Closed dcaratti closed 1 year ago

dcaratti commented 1 year ago

when packetdrill builds a cmsg parsed from the script, it handles 'ee_origin', 'ee_type' and 'ee_code' as 32-bit numbers. These members of 'struct sock_extended_err' are 8-bit wide: because of this, TCP tests belonging to 'timestamping' and 'zerocopy' categories systematically fail on big endian architectures (such as s390x). For example, the value of 'ee_origin' is written in 'ee_pad':

(gdb) set args -D TFO_COOKIE=de4f234f0f433a55 -D CMSG_LEVEL_IP=SOL_IP -D CMSG_TYPE_RECVERR=IP_RECVERR basic.pkt (gdb) r

[...]

24: system call: recvmsg 24: invoke call: recvmsg waiting until 1668090937472983 -- now is 1668090937472985 expected: 0.000 actual: 0.000 (secs)

Thread 1 "packetdrill" hit Breakpoint 2, new_extended_err (expr=0x111e7a0, ee=0x1124d80, error=0x3ffffff9730) at run_system_call.c:500 500 if (get_s32(expr->ee_errno, (s32 )&ee->ee_errno, error)) (gdb) n 502 if (get_s32(expr->ee_origin, (s32 )&ee->ee_origin, error)) (gdb) n 504 if (get_s32(expr->ee_type, (s32 )&ee->ee_type, error)) (gdb) p ee $46 = {ee_errno = 0, ee_origin = 0 '\000', ee_type = 0 '\000', ee_code = 0 '\000', ee_pad = 5 '\005', ee_info = 0, ee_data = 0} (gdb) up

1 0x000000000101d892 in cmsg_new (expr=0x111f740, msg=0x1124c60, error=0x3ffffff9730) at run_system_call.c:617

617 if (new_extended_err(ee_expr, (gdb) p (struct sock_extended_err )data $47 = {ee_errno = 0, ee_origin = 0 '\000', ee_type = 0 '\000', ee_code = 0 '\000', ee_pad = 5 '\005', ee_info = 0, ee_data = 0} (gdb)

fix it by providing a correct integer size for 'ee_origin', 'ee_type' and 'ee_code'.

Reported-by: Xiumei Mu xmu@redhat.com Signed-off-by: Davide Caratti dcaratti@redhat.com

dcaratti commented 1 year ago

Thanks for the fix!

For consistency/clarity, can you please make the summary/first-line of the commit message look like:

net-test: packetdrill: fix access to 'sock_extended_err' when generating cmsg from expressions

Thanks!

hi Neal, I just fixed the commit message. Thanks for reviewing!

dcaratti commented 1 year ago

The packetdrill tree uses Linux's scripts/checkpatch.pl for style validation, and it caught some issues (see below).

Ouch, sorry for the copy/paste noise.

I will fix those issues and the commit title issue, and merge if all our internal tests pass. Thanks!

Ok; for your convenience, I refreshed the PR with fixed indentation. Thanks!