Print

Print


When `ProcessPushReq()` and `ProcessPullReq()` leave scope, `TPC::State::~State()` is called. The destructor modifies the curl handle, but the handle may already have been through `curl_easy_cleanup()`, and any further use may cause memory corruption.

Make the curl handle NULL after `curl_easy_cleanup()`.

<details>
<summary>Valgrind output from xrootd-5.1.2-0.experimental.2534824.4ca38eb6.el7.cern.x86_64 and curl-7.29.0-59.el7_9.1.x86_64</summary>

```
-bash-4.2$ valgrind --undef-value-errors=no /usr/bin/xrootd -l /var/log/xrootd/xrootd.log -c /etc/xrootd/xrootd-clustered.cfg -k fifo -s /var/run/xrootd/xrootd-clustered.pid -n clustered
==20804== Memcheck, a memory error detector
==20804== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==20804== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==20804== Command: /usr/bin/xrootd -l /var/log/xrootd/xrootd.log -c /etc/xrootd/xrootd-clustered.cfg -k fifo -s /var/run/xrootd/xrootd-clustered.pid -n clustered
==20804==
==20804== Thread 52:
==20804== Invalid write of size 8
==20804==    at 0x127A5173: Curl_setopt (url.c:1098)
==20804==    by 0x127B373A: curl_easy_setopt (easy.c:384)
==20804==    by 0x1608349D: TPC::State::~State() (XrdTpcState.cc:22)
==20804==    by 0x16095C0D: TPC::TPCHandler::ProcessPullReq(std::string const&, XrdHttpExtReq&) (XrdTpcTPC.cc:777)
==20804==    by 0x1609688D: TPC::TPCHandler::ProcessReq(XrdHttpExtReq&) (XrdTpcTPC.cc:147)
==20804==    by 0xB2FFF82: XrdHttpReq::ProcessHTTPReq() (XrdHttpReq.cc:998)
==20804==    by 0xB2F7C7C: XrdHttpProtocol::Process(XrdLink*) (XrdHttpProtocol.cc:854)
==20804==    by 0x51D6AA5: XrdLinkXeq::DoIt() (XrdLinkXeq.cc:319)
==20804==    by 0x51D31D8: XrdLink::setProtocol(XrdProtocol*, bool, bool) (XrdLink.cc:435)
==20804==    by 0x51D9DA5: XrdScheduler::Run() (XrdScheduler.cc:382)
==20804==    by 0x51D9EF8: XrdStartWorking(void*) (XrdScheduler.cc:88)
==20804==    by 0x517F846: XrdSysThread_Xeq (XrdSysPthread.cc:86)
==20804==  Address 0x2334a0f0 is 864 bytes inside a block of size 19,536 free'd
==20804==    at 0x4C2B06D: free (vg_replace_malloc.c:540)
==20804==    by 0x127A40FC: Curl_close (url.c:471)
==20804==    by 0x1609131E: TPC::TPCHandler::RunCurlWithUpdates(void*, XrdHttpExtReq&, TPC::State&, TPC::TPCHandler::TPCLogRecord&) (XrdTpcTPC.cc:514)
==20804==    by 0x16095F40: TPC::TPCHandler::ProcessPullReq(std::string const&, XrdHttpExtReq&) (XrdTpcTPC.cc:784)
==20804==    by 0x1609688D: TPC::TPCHandler::ProcessReq(XrdHttpExtReq&) (XrdTpcTPC.cc:147)
==20804==    by 0xB2FFF82: XrdHttpReq::ProcessHTTPReq() (XrdHttpReq.cc:998)
==20804==    by 0xB2F7C7C: XrdHttpProtocol::Process(XrdLink*) (XrdHttpProtocol.cc:854)
==20804==    by 0x51D6AA5: XrdLinkXeq::DoIt() (XrdLinkXeq.cc:319)
==20804==    by 0x51D31D8: XrdLink::setProtocol(XrdProtocol*, bool, bool) (XrdLink.cc:435)
==20804==    by 0x51D9DA5: XrdScheduler::Run() (XrdScheduler.cc:382)
==20804==    by 0x51D9EF8: XrdStartWorking(void*) (XrdScheduler.cc:88)
==20804==    by 0x517F846: XrdSysThread_Xeq (XrdSysPthread.cc:86)
==20804==  Block was alloc'd at
==20804==    at 0x4C2C089: calloc (vg_replace_malloc.c:762)
==20804==    by 0x127A4369: Curl_open (url.c:604)
==20804==    by 0x127B3663: curl_easy_init (easy.c:358)
==20804==    by 0x160952C0: TPC::TPCHandler::ProcessPullReq(std::string const&, XrdHttpExtReq&) (XrdTpcTPC.cc:704)
==20804==    by 0x1609688D: TPC::TPCHandler::ProcessReq(XrdHttpExtReq&) (XrdTpcTPC.cc:147)
==20804==    by 0xB2FFF82: XrdHttpReq::ProcessHTTPReq() (XrdHttpReq.cc:998)
==20804==    by 0xB2F7C7C: XrdHttpProtocol::Process(XrdLink*) (XrdHttpProtocol.cc:854)
==20804==    by 0x51D6AA5: XrdLinkXeq::DoIt() (XrdLinkXeq.cc:319)
==20804==    by 0x51D31D8: XrdLink::setProtocol(XrdProtocol*, bool, bool) (XrdLink.cc:435)
==20804==    by 0x51D9DA5: XrdScheduler::Run() (XrdScheduler.cc:382)
==20804==    by 0x51D9EF8: XrdStartWorking(void*) (XrdScheduler.cc:88)
==20804==    by 0x517F846: XrdSysThread_Xeq (XrdSysPthread.cc:86)
==20804==
```
</details>
You can view, comment on, or merge this pull request online at:

  https://github.com/xrootd/xrootd/pull/1449

-- Commit Summary --

  * [XrdTpc] Do not modify curl handle after curl_easy_cleanup()

-- File Changes --

    M src/XrdTpc/XrdTpcTPC.cc (7)

-- Patch Links --

https://github.com/xrootd/xrootd/pull/1449.patch
https://github.com/xrootd/xrootd/pull/1449.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/xrootd/xrootd/pull/1449

########################################################################
Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1