[LU-12605] Lustre target_handle_connect() bug

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.13.0
• Fix Version/s: Lustre 2.13.0, Lustre 2.12.3
• Type: Bug
• Priority: Critical
• Reporter: Alibaba Cloud
• Assignee: Emoly Liu
• Resolution: Fixed
• Severity: 3

Description

In the latest version of lustre file system, ptlrpc module has a buffer overflow bug due to the lack of validation for specific fields of packets sent by client.

The kenrel panic:

[  607.979453] Call Trace:
[  607.981190]  [<ffffffffc0a76199>] ? +0xd19/0x2960 [ptlrpc]
[  607.983385]  [<ffffffffc0b1f02a>] tgt_request_handle+0x67a/0x15c0 [ptlrpc]
[  607.985484]  [<ffffffffc0710fa7>] ? libcfs_debug_msg+0x57/0x80 [libcfs]
[  607.987581]  [<ffffffffc0ac288e>] ptlrpc_server_handle_request+0x24e/0xab0 [ptlrpc]
[  607.989741]  [<ffffffffabacbadb>] ? __wake_up_common+0x5b/0x90
[  607.991741]  [<ffffffffc0ac6384>] ptlrpc_main+0xbb4/0x20f0 [ptlrpc]
[  607.993731]  [<ffffffffabad08c0>] ? finish_task_switch+0x50/0x1c0
[  607.995760]  [<ffffffffc0ac57d0>] ? ptlrpc_register_service+0xfa0/0xfa0 [ptlrpc]
[  607.997834]  [<ffffffffabac1c71>] kthread+0xd1/0xe0
[  607.999655]  [<ffffffffabac1ba0>] ? insert_kthread_work+0x40/0x40
[  608.001584]  [<ffffffffac175c1d>] ret_from_fork_nospec_begin+0x7/0x21
[  608.003533]  [<ffffffffabac1ba0>] ? insert_kthread_work+0x40/0x40

The function target_handle_connect() don't check the value of size when client connect to server. If size is -1, the min function will return -1. But the third parameter of memcpy is unsigned int, -1 will be parsed into 0xffffffff, causing a buffer overflow.

size = req_capsule_get_size(&req->rq_pill, &RMF_CONNECT_DATA,
                                    RCL_CLIENT);
memcpy(tmpdata, data, min(tmpsize, size));

 

 

Issue Links

• is related to: LU-12590 Lustre lustre_msg_hdr_size_v2() bug
• is related to: LU-12600 Lustre tgt_brw_write() bug
• is related to: LU-12602 Lustre mdt_getxattr_pack_reply() bug
• is related to: LU-12603 Lustre ldlm_request_cancel() bug
• is related to: LU-12604 Lustre mdt_file_secctx_unpack() bug
• is related to: LU-12612 Lustre osd_bufs_get() bug
• is related to: LU-12613 Lustre lustre_msg_string() bug
• is related to: LU-12615 Lustre mdt_object_remote() bug

Comments

[Peter Jones]

Sébastien

Could you please investigate

Peter

[Andreas Dilger]

lustre_unpack_msg_v2() is what is touching the message buffer first, and is the right place to do sanity checking of the message buffers. While lustre_msg_buf_v2() is used in a lot of places to access various buffers internally, we don't necessarily want to do validity checking for every access throughout the code. Having a high-level scrub once at message receipt makes the most sense, and then the internal accessors can assume it is valid.

Checks that should be added:

1. validate that the number of buffers is not too large (there a limit of 32 buffers per message based on sizeof(req->rq_req_swab_mask)*8, but there is no check of lm_bufcount before it is used.
2. validate that individual buffer lengths and the total message lengths are not insane (i.e. integers < INT_MAX/MAX_BUFFER_COUNT so that they are not treated as negative int values)
3. validate that the individual buffer lengths are less than the total message length to avoid overflow when added
4. validate that the provided lengths are not larger than the total message size

[Peter Jones]

Emoly

Actually can you please work on this one

Thanks

Peter

[Andreas Dilger]

Hello yunye.ry,

please register for an account on Gerrit https://review.whamcloud.com/ so that you can be added as a reviewer for the patches that are being developed for the various issues that you have filed.  Then you can verify that they are fixing the reported problems by adding your +1 to the patch (assuming it is correct).  This will result in a "Reviewed-by: Your Name <your_email>" label on the final patch, and Alibaba will also be listed in the Lustre contribution statistics in Lustre presentations.

[Alibaba Cloud]

Ok, I will do it, thank you

[Alibaba Cloud]

Hi Andreas,
I tried to register in the https://review.whamcloud.com/ but failed. The register site ( https://review.whamcloud.com/login/%23%2Fregister%2Fq%2Fstatus%3Aopen) shows that I can log in with my whamcloud account, but it seems failed. The wrong message is 'Invalid authentication'. Do I have to register for an OpenID account? Thank you.

[Peter Jones]

Yes - as you are not part of the Whamcloud team you will need an account based on an OpenID identity. These kinds of questions are probably best handled via direct email rather than in JIRA (where anyone can read them)

[Gerrit Updater]

Emoly Liu ([e-mail]) uploaded a new patch: https://review.whamcloud.com/35711
Subject: LU-12605 tgt: check data size in target_handle_connect()
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: ba4979f6b25e64576e7906ec6cd559f3499adba7

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/35711/
Subject: LU-12605 tgt: check client data size in target_handle_connect()
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 149f005a3199eee13fe6396671613a0f620ee0cc

[Peter Jones]

Landed for 2.13

[Gerrit Updater]

Minh Diep ([e-mail]) uploaded a new patch: https://review.whamcloud.com/35935
Subject: LU-12605 tgt: check client data size in target_handle_connect()
Project: fs/lustre-release
Branch: b2_12
Current Patch Set: 1
Commit: c43893e5a876a180b321df714a61630ad4b7c03f

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/35935/
Subject: LU-12605 tgt: check client data size in target_handle_connect()
Project: fs/lustre-release
Branch: b2_12
Current Patch Set:
Commit: d70c45a124aa2580115111aa8a77648f073dc799