Copilot commented on code in PR #13027:
URL: https://github.com/apache/trafficserver/pull/13027#discussion_r2999005908
##########
src/api/InkAPI.cc:
##########
@@ -7067,11 +7067,13 @@ TSAIORead(int fd, off_t offset, char *buf, size_t
buffSize, TSCont contp)
pAIO->aiocb.aio_buf = buf;
pAIO->action = pCont;
pAIO->thread = pCont->mutex->thread_holding;
+ pAIO->from_api = true;
if (ink_aio_read(pAIO, 1) == 1) {
return TS_SUCCESS;
}
+ delete pAIO;
return TS_ERROR;
Review Comment:
There are now two sources of truth for 'API-originated' AIO (`from_api` on
the object, and the existing `fromAPI` parameter passed to
`ink_aio_read/write`), which can drift and reintroduce leaks if a future API
path passes `fromAPI=1` but forgets to set `from_api`. Prefer a single
mechanism: set `op_in->from_api` based on the `fromAPI` argument inside
`ink_aio_read/write` (or `aio_queue_req`) and remove the need for callers to
set `from_api` explicitly.
##########
src/proxy/http/HttpProxyServerMain.cc:
##########
@@ -376,4 +376,16 @@ stop_HttpProxyServer()
{
sslNetProcessor.stop_accept();
netProcessor.stop_accept();
+
+ for (auto &acceptor : HttpProxyAcceptors) {
+ delete acceptor._accept;
+ acceptor._accept = nullptr;
+ }
+ HttpProxyAcceptors.clear();
+
+ delete plugin_http_accept;
+ plugin_http_accept = nullptr;
+
+ delete plugin_http_transparent_accept;
Review Comment:
Potential double-free/UAF: this function now deletes acceptor objects after
calling `sslNetProcessor.stop_accept()` / `netProcessor.stop_accept()`, but
`UnixNetProcessor::stop_accept()` was changed in this PR to delete its tracked
`NetAccept` objects. If `acceptor._accept`, `plugin_http_accept`, or
`plugin_http_transparent_accept` are among those tracked, they may already be
freed by `stop_accept()`. Fix by establishing a single owner: either (a) let
`*NetProcessor::stop_accept()` own and delete acceptors and here only
null/clear pointers without `delete`, or (b) keep deletion here and revert
`stop_accept()` to not delete (less desirable given leak fix).
```suggestion
acceptor._accept = nullptr;
}
HttpProxyAcceptors.clear();
plugin_http_accept = nullptr;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
