On Thu, Apr 02, 2020 at 06:29:06PM +0300, Denis Kovalchuk wrote: > Hello. > > I think I have found an undefined behavior in the code that constructs ID for > per-transaction DAG node cache. > > In make_txn_root() function, the txn variable of type svn_fs_fs__id_part_t * > is passed to apr_pstrcat() function, leading to the undefined behavior. > I can assume that originally it was planned to pass a textual representation > of the txn, instead of passing the txn itself. > > Although ID is not used anywhere, except debug-only calls of > svn_cache__get_info() and svn_cache__format_info() functions, the improper > call > to apr_pstrcat() may cause a potential crash, etc. > > I have attached a patch that fixes it. > > Regards, > Denis Kovalchuk
Thank you. Committed in r1876054 (I trimmed one line in your log message that was longer than 80 coloumns wide). I will also nominate this for backports. > Fix undefined behavior when constructing ID for txn_node_cache in fsfs. > > In make_txn_root() function, the txn variable of type svn_fs_fs__id_part_t * > is passed to apr_pstrcat() function, leading to the undefined behavior. > I can assume that originally it was planned to pass a textual representation > of the txn, instead of passing the txn itself. > > Although ID is not used anywhere, except debug-only calls of > svn_cache__get_info() and svn_cache__format_info() functions, the improper > call > to apr_pstrcat() may cause a potential crash, etc. > > * subversion/libsvn_fs_fs/tree.c > (make_txn_root): Pass a textual representation of the txn to apr_pstrcat() > function. > > Patch by: Denis Kovalchuk <denis.kovalc...@visualsvn.com> > > Index: subversion/libsvn_fs_fs/tree.c > =================================================================== > --- subversion/libsvn_fs_fs/tree.c (revision 1875922) > +++ subversion/libsvn_fs_fs/tree.c (working copy) > @@ -4644,7 +4644,7 @@ make_txn_root(svn_fs_root_t **root_p, > svn_fs_fs__dag_deserialize, > APR_HASH_KEY_STRING, > 32, 20, FALSE, > - apr_pstrcat(pool, txn, ":TXN", > + apr_pstrcat(pool, root->txn, ":TXN", > SVN_VA_NULL), > root->pool)); >
