Re: Subversion's Berkeley DB detection with APR trunk
On 06/21/2011 09:42 AM, Philip Martin wrote: > "C. Michael Pilato" writes: > >> I'd be interested in knowing why the --db-version parameter was >> dropped. In the absence of a solid reason, I think APR should be >> changed to support as much. > > Probably it's because apr is now modular and BDB support is one of the > things that is loaded dynamically. It's easy enough to reinstate > --db-version, but that doesn't fix Subversion because apr no longer > explicitly links against the BDB library. Subversion's configure fails > its link test with 'undefined reference to db_version'. Hrm. I seem to recall that it used to be extremely important for Subversion to use the same BDB to which APR was linked. mod_dav_svn used APR's DB support for its activities database, and couldn't deal with having APR's DB being one flavor of BDB while the Subversion repositories which it also needed to read/write needed a different flavor of BDB. Has any of this changed since our move (back in 1.5) away from DB-based activities databases? I mean, could we move BDB detection into Subversion proper and that not be catastrophic? -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Subversion's Berkeley DB detection with APR trunk
On 06/21/2011 08:03 AM, Philip Martin wrote: > apr trunk combines apr and apr-util into a single library and there is > only a single apr-2-config script. Subversion can be configured by > using this script for both apr and apr-util, but Subversion's Berkeley > DB detection doesn't work. This is because the apr-2-config script > doesn't support the --db-version parameter that the old apu-config > script provided. > > Should we rework Subversion's Berkeley DB detection or should we change > APR? I'd be interested in knowing why the --db-version parameter was dropped. In the absence of a solid reason, I think APR should be changed to support as much. -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand
Re: Buffer overflow in apr_brigade_vprintf() ?
C. Michael Pilato wrote: > [Please Cc: me in responses -- I think I still have APR commit privs, but > I'm not active here and not subscribed to the mailing lists.] > > In the past couple of weeks, I've seen two different reports of what appears > to be corruption in the stream of data transmitted by Subversion's > mod_dav_svn through Apache and back to the Subversion client. What is seen > client-side is an opening XML tag, a truncated bit of CDATA "inside" the > tag, and then a missing XML closing tag. The problem seems to occur with > magically sized chunks of data, so it can be hard to reproduce[1]. [...] Just to bring this to closure, the bug was fixed by committing the removal of the code that tacks the NULL byte onto a possibly-already-full buffer: http://svn.apache.org/viewvc?view=rev&revision=768417 (Thanks, Ruediger and Jeff!) -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Buffer overflow in apr_brigade_vprintf() ?
[Please Cc: me in responses -- I think I still have APR commit privs, but
I'm not active here and not subscribed to the mailing lists.]
In the past couple of weeks, I've seen two different reports of what appears
to be corruption in the stream of data transmitted by Subversion's
mod_dav_svn through Apache and back to the Subversion client. What is seen
client-side is an opening XML tag, a truncated bit of CDATA "inside" the
tag, and then a missing XML closing tag. The problem seems to occur with
magically sized chunks of data, so it can be hard to reproduce[1].
Here are the relevant pieces of the call stack:
mod_dav_svn/reports/replay.c's change_file_or_dir_prop() function contains
the following (which is base64-encoding Subversion file and directory
properties, and tossing them into an XML REPORT request response):
const svn_string_t *enc_value = svn_base64_encode_string2(value, TRUE,
pool);
SVN_ERR(dav_svn__send_xml
(eb->bb, eb->output,
"%s" DEBUG_CR,
file_or_dir, qname, enc_value->data, file_or_dir));
dav_svn__send_xml() is a wrapper around apr_brigade_vprintf().
As you know, apr_brigade_vprintf() (in buckets/apr_brigade.c) looks like so:
APU_DECLARE(apr_status_t) apr_brigade_vprintf(apr_bucket_brigade *b,
apr_brigade_flush flush,
void *ctx,
const char *fmt, va_list va)
{
/* the cast, in order of appearance */
struct brigade_vprintf_data_t vd;
char buf[APR_BUCKET_BUFF_SIZE];
int written;
vd.vbuff.curpos = buf;
vd.vbuff.endpos = buf + APR_BUCKET_BUFF_SIZE;
vd.b = b;
vd.flusher = &flush;
vd.ctx = ctx;
vd.cbuff = buf;
written = apr_vformatter(brigade_flush, &vd.vbuff, fmt, va);
if (written == -1) {
return -1;
}
/* tack on null terminator to remaining string */
*(vd.vbuff.curpos) = '\0';
/* write out what remains in the buffer */
return apr_brigade_write(b, flush, ctx, buf, vd.vbuff.curpos - buf);
}
The function apr_vformatter() uses the buffer "buf" to format the string.
This function in turn uses the macro INS_CHAR to add characters to the
buffer. INS_CHAR is defined like this:
#define INS_CHAR(c, sp, bep, cc)\
{ \
if (sp) { \
if (sp >= bep) {\
vbuff->curpos = sp; \
if (flush_func(vbuff)) \
return -1; \
sp = vbuff->curpos; \
bep = vbuff->endpos;\
} \
*sp++ = (c);\
} \
cc++; \
}
So, when the macro is executed to add a new character to the buffer and the
buffer is full, the flush function is called to make room for the new
character, and then the character is added. Of course, if the buffer has
room for exactly one more character, it is not flushed, the character is
added, and the current position of the buffer is at its end (which is
actually one byte beyond the space allocated for the buffer).
After the call to apr_vformatter(), there will be stuff in the buffer. In
the special case above, the buffer may be perfectly full (perhaps after
having been flushed one or more times, but still full now). Then, without
checking for that condition, this line is executed:
/* tack on null terminator to remaining string */
*(vd.vbuff.curpos) = '\0';
Uh-oh. Buffer overflow!
Our CollabNet engineer is proposing a simple fix: defining 'buf' inside
apr_brigade_vprintf() like so:
char buf[APR_BUCKET_BUFF_SIZE + 1]
(Note the "+ 1" to make room for that pesky NULL byte.)
But I'm wondering if an equally correct fix would be to simply not tack the
'\0' onto the buffer at all. Doesn't apr_brigade_write() accept both the
buffer and the number of bytes to write? Does it really need a
null-terminated string, especially considering that its input could be
arbitrary binary data? Other calls to it pass things like "str" and
"strlen(str)", which would ignore the NULL byte in "str".
[1]
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&viewType=browseAll&dsMessageId=1745697
--
C. Michael Pilato
CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
Re: confusion about largefile support
Erik Huelsmann <[EMAIL PROTECTED]> writes: > On 5/31/05, Ben Collins-Sussman <[EMAIL PROTECTED]> wrote: > > > > On May 31, 2005, at 11:49 AM, Ben Collins-Sussman wrote: > > > > > > Funny, KDE is using fsfs, and I would have expected them to run > > > into a >2GB revision file. > > > > > > > Well, whattya know. Now Timothee Besset (ttimo) in IRC has just > > reported the same "File size limit exceeded" error that we saw on > > users@ earlier today. In both cases, the users were loading a > > dumpfile into an fsfs repository. And ttimo verified my fear. > > There's a >2GB file being assembled in db/txns/. > > > > So, um, maybe we should write a FAQ? One which tells folks that the > > only workaround here is to recompile subversion against apr 1.x? > > (And to upgrade to httpd 2.1 if necessary.) > > or use a BDB repos. In TTimo's case, I seem to recall that the use of a BDB repos was a cause of entirely different source of pain, and therefore, not as viable an option.
Re: uuid generation on linux boxes
Ian Holsman <[EMAIL PROTECTED]> writes: > I have a issue with a linux box not having enough entropy and hanging > on a call to apr_uuid_get (as it calls /dev/random) > > I was wondering why we don't just use /proc/sys/kernel/random/uuid > and use that for our uuid? If you are building APR yourself, I believe you can pass --with-devrandom=/dev/urandom to its configure script to solve this problem.
