Author: svn-role Date: Fri Sep 20 04:00:21 2019 New Revision: 1867194 URL: http://svn.apache.org/viewvc?rev=1867194&view=rev Log: Merge r1850651 from trunk:
* r1850651 Fix a use-after-free in mod_dav_svn's logging of FS warnings. Justification: Lots of crashes on OpenBSD during 'make check' with HTTPD 2.4. See https://svn.haxx.se/dev/archive-2018-12/0137.shtml Votes: +1: stsp, brane Modified: subversion/branches/1.11.x/ (props changed) subversion/branches/1.11.x/STATUS subversion/branches/1.11.x/subversion/mod_dav_svn/repos.c Propchange: subversion/branches/1.11.x/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Fri Sep 20 04:00:21 2019 @@ -100,4 +100,4 @@ /subversion/branches/verify-at-commit:1462039-1462408 /subversion/branches/verify-keep-going:1439280-1546110 /subversion/branches/wc-collate-path:1402685-1480384 -/subversion/trunk:1840990-1840991,1840995,1840997,1841059,1841079,1841091,1841098,1841136,1841180,1841272,1841481,1841524-1841525,1841567,1841600-1841602,1841606,1841719,1841725,1841731,1841736,1841742-1841743,1841753-1841754,1841822,1841850,1841867,1842090,1842222-1842223,1842334,1842814,1842827,1842829,1842877,1843888,1844882,1844987,1845204,1845212,1845261,1845408,1845555-1845556,1845559,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847377,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621,1851676,1851687,1851791,1853761 +/subversion/trunk:1840990-1840991,1840995,1840997,1841059,1841079,1841091,1841098,1841136,1841180,1841272,1841481,1841524-1841525,1841567,1841600-1841602,1841606,1841719,1841725,1841731,1841736,1841742-1841743,1841753-1841754,1841822,1841850,1841867,1842090,1842222-1842223,1842334,1842814,1842827,1842829,1842877,1843888,1844882,1844987,1845204,1845212,1845261,1845408,1845555-1845556,1845559,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847377,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621,1850651,1851676,1851687,1851791,1853761 Modified: subversion/branches/1.11.x/STATUS URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/STATUS?rev=1867194&r1=1867193&r2=1867194&view=diff ============================================================================== --- subversion/branches/1.11.x/STATUS (original) +++ subversion/branches/1.11.x/STATUS Fri Sep 20 04:00:21 2019 @@ -69,14 +69,6 @@ Veto-blocked changes: Approved changes: ================= - * r1850651 - Fix a use-after-free in mod_dav_svn's logging of FS warnings. - Justification: - Lots of crashes on OpenBSD during 'make check' with HTTPD 2.4. - See https://svn.haxx.se/dev/archive-2018-12/0137.shtml - Votes: - +1: stsp, brane - * r1852013 Fix issue #4804: avoid test failures just because some SQLite compile-time feature was enabled that changes its query plan descriptions. Modified: subversion/branches/1.11.x/subversion/mod_dav_svn/repos.c URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/mod_dav_svn/repos.c?rev=1867194&r1=1867193&r2=1867194&view=diff ============================================================================== --- subversion/branches/1.11.x/subversion/mod_dav_svn/repos.c (original) +++ subversion/branches/1.11.x/subversion/mod_dav_svn/repos.c Fri Sep 20 04:00:21 2019 @@ -1225,25 +1225,32 @@ create_private_resource(const dav_resour return &comb->res; } - -static void log_warning(void *baton, svn_error_t *err) +static void log_warning_req(void *baton, svn_error_t *err) { request_rec *r = baton; const char *continuation = ""; - /* ### hmm. the FS is cleaned up at request cleanup time. "r" might - ### not really be valid. we should probably put the FS into a - ### subpool to ensure it gets cleaned before the request. + /* Not showing file/line so no point in tracing */ + err = svn_error_purge_tracing(err); + while (err) + { + ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r, "%s%s", + continuation, err->message); + continuation = "-"; + err = err->child; + } +} - ### is there a good way to create and use a subpool for all - ### of our functions ... ?? - */ +static void log_warning_conn(void *baton, svn_error_t *err) +{ + conn_rec *c = baton; + const char *continuation = ""; /* Not showing file/line so no point in tracing */ err = svn_error_purge_tracing(err); while (err) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r, "%s%s", + ap_log_cerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, c, "%s%s", continuation, err->message); continuation = "-"; err = err->child; @@ -1547,6 +1554,24 @@ cleanup_fs_access(void *data) return APR_SUCCESS; } +/* Context for cleanup handler. */ +struct cleanup_req_logging_baton +{ + svn_fs_t *fs; + conn_rec *connection; +}; + +static apr_status_t +cleanup_req_logging(void *data) +{ + struct cleanup_req_logging_baton *baton = data; + + /* The request about to be freed. Log future warnings with a connection + * context instead of a request context. */ + svn_fs_set_warning_func(baton->fs, log_warning_conn, baton->connection); + + return APR_SUCCESS; +} /* Helper func to construct a special 'parentpath' private resource. */ static dav_error * @@ -2180,6 +2205,7 @@ get_resource(request_rec *r, int had_slash; dav_locktoken_list *ltl; struct cleanup_fs_access_baton *cleanup_baton; + struct cleanup_req_logging_baton *cleanup_req_logging_baton; void *userdata; apr_hash_t *fs_config; @@ -2486,7 +2512,7 @@ get_resource(request_rec *r, repos->fs = svn_repos_fs(repos->repos); /* capture warnings during cleanup of the FS */ - svn_fs_set_warning_func(repos->fs, log_warning, r); + svn_fs_set_warning_func(repos->fs, log_warning_req, r); /* if an authenticated username is present, attach it to the FS */ if (r->user) @@ -2503,6 +2529,14 @@ get_resource(request_rec *r, apr_pool_cleanup_register(r->pool, cleanup_baton, cleanup_fs_access, apr_pool_cleanup_null); + /* We must degrade the logging context when the request is freed. */ + cleanup_req_logging_baton = + apr_pcalloc(r->pool, sizeof(*cleanup_req_logging_baton)); + cleanup_req_logging_baton->fs = repos->fs; + cleanup_req_logging_baton->connection = r->connection; + apr_pool_pre_cleanup_register(r->pool, cleanup_req_logging_baton, + cleanup_req_logging); + /* Create an access context based on the authenticated username. */ serr = svn_fs_create_access(&access_ctx, r->user, r->pool); if (serr)