<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/libgit2.git/src/buffer.c, branch ethomson/readme</title>
<subtitle>github.com: libgit2/libgit2.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/'/>
<entry>
<title>buffer: git_buf_copy_cstr should return a value</title>
<updated>2020-11-25T11:42:03+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2020-04-05T10:22:19+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=e0f3c33d5a505f5341300351c2fcc493929a932f'/>
<id>e0f3c33d5a505f5341300351c2fcc493929a932f</id>
<content type='text'>
`git_buf_copy_cstr` is called with user-input, and wants to sanity-check
that input.  Allow it to return a value if the input was malformed in a
way that we cannot cope.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
`git_buf_copy_cstr` is called with user-input, and wants to sanity-check
that input.  Allow it to return a value if the input was malformed in a
way that we cannot cope.
</pre>
</div>
</content>
</entry>
<entry>
<title>buffer: git_buf_sanitize should return a value</title>
<updated>2020-11-25T11:42:03+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2020-04-05T10:07:54+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=cb4bfbc99dffa7679f42cf8500931fa250bb7db3'/>
<id>cb4bfbc99dffa7679f42cf8500931fa250bb7db3</id>
<content type='text'>
`git_buf_sanitize` is called with user-input, and wants to sanity-check
that input.  Allow it to return a value if the input was malformed in a
way that we cannot cope.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
`git_buf_sanitize` is called with user-input, and wants to sanity-check
that input.  Allow it to return a value if the input was malformed in a
way that we cannot cope.
</pre>
</div>
</content>
</entry>
<entry>
<title>buffer: use GIT_ASSERT</title>
<updated>2020-11-25T11:42:03+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2020-04-05T09:23:20+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=a6dd58659d16207ec92a1f4d87ec620236ce4a23'/>
<id>a6dd58659d16207ec92a1f4d87ec620236ce4a23</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Make the tests run cleanly under UndefinedBehaviorSanitizer</title>
<updated>2020-06-30T13:23:06+00:00</updated>
<author>
<name>lhchavez</name>
<email>lhchavez@lhchavez.com</email>
</author>
<published>2020-06-27T19:15:26+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=d0656ac80f33d54967425e782cbe71a0f1b963db'/>
<id>d0656ac80f33d54967425e782cbe71a0f1b963db</id>
<content type='text'>
This change makes the tests run cleanly under
`-fsanitize=undefined,nullability` and comprises of:

* Avoids some arithmetic with NULL pointers (which UBSan does not like).
* Avoids an overflow in a shift, due to an uint8_t being implicitly
  converted to a signed 32-bit signed integer after being shifted by a
  32-bit signed integer.
* Avoids a unaligned read in libgit2.
* Ignores unaligned reads in the SHA1 library, since it only happens on
  Intel processors, where it is _still_ undefined behavior, but the
  semantics are moderately well-understood.

Of notable omission is `-fsanitize=integer`, since there are lots of
warnings in zlib and the SHA1 library which probably don't make sense to
fix and I could not figure out how to silence easily. libgit2 itself
also has ~100s of warnings which are mostly innocuous (e.g. use of enum
constants that only fit on an `uint32_t`, but there is no way to do that
in a simple fashion because the data type chosen for enumerated types is
implementation-defined), and investigating whether there are worrying
warnings would need reducing the noise significantly.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This change makes the tests run cleanly under
`-fsanitize=undefined,nullability` and comprises of:

* Avoids some arithmetic with NULL pointers (which UBSan does not like).
* Avoids an overflow in a shift, due to an uint8_t being implicitly
  converted to a signed 32-bit signed integer after being shifted by a
  32-bit signed integer.
* Avoids a unaligned read in libgit2.
* Ignores unaligned reads in the SHA1 library, since it only happens on
  Intel processors, where it is _still_ undefined behavior, but the
  semantics are moderately well-understood.

Of notable omission is `-fsanitize=integer`, since there are lots of
warnings in zlib and the SHA1 library which probably don't make sense to
fix and I could not figure out how to silence easily. libgit2 itself
also has ~100s of warnings which are mostly innocuous (e.g. use of enum
constants that only fit on an `uint32_t`, but there is no way to do that
in a simple fashion because the data type chosen for enumerated types is
implementation-defined), and investigating whether there are worrying
warnings would need reducing the noise significantly.
</pre>
</div>
</content>
</entry>
<entry>
<title>tree-wide: do not compile deprecated functions with hard deprecation</title>
<updated>2020-06-09T12:57:06+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2020-06-08T19:07:36+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=c6184f0c4b209e462bf3f42ab20df2d13d8ee918'/>
<id>c6184f0c4b209e462bf3f42ab20df2d13d8ee918</id>
<content type='text'>
When compiling libgit2 with -DDEPRECATE_HARD, we add a preprocessor
definition `GIT_DEPRECATE_HARD` which causes the "git2/deprecated.h"
header to be empty. As a result, no function declarations are made
available to callers, but the implementations are still available to
link against. This has the problem that function declarations also
aren't visible to the implementations, meaning that the symbol's
visibility will not be set up correctly. As a result, the resulting
library may not expose those deprecated symbols at all on some platforms
and thus cause linking errors.

Fix the issue by conditionally compiling deprecated functions, only.
While it becomes impossible to link against such a library in case one
uses deprecated functions, distributors of libgit2 aren't expected to
pass -DDEPRECATE_HARD anyway. Instead, users of libgit2 should manually
define GIT_DEPRECATE_HARD to hide deprecated functions. Using "real"
hard deprecation still makes sense in the context of CI to test we don't
use deprecated symbols ourselves and in case a dependant uses libgit2 in
a vendored way and knows it won't ever use any of the deprecated symbols
anyway.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When compiling libgit2 with -DDEPRECATE_HARD, we add a preprocessor
definition `GIT_DEPRECATE_HARD` which causes the "git2/deprecated.h"
header to be empty. As a result, no function declarations are made
available to callers, but the implementations are still available to
link against. This has the problem that function declarations also
aren't visible to the implementations, meaning that the symbol's
visibility will not be set up correctly. As a result, the resulting
library may not expose those deprecated symbols at all on some platforms
and thus cause linking errors.

Fix the issue by conditionally compiling deprecated functions, only.
While it becomes impossible to link against such a library in case one
uses deprecated functions, distributors of libgit2 aren't expected to
pass -DDEPRECATE_HARD anyway. Instead, users of libgit2 should manually
define GIT_DEPRECATE_HARD to hide deprecated functions. Using "real"
hard deprecation still makes sense in the context of CI to test we don't
use deprecated symbols ourselves and in case a dependant uses libgit2 in
a vendored way and knows it won't ever use any of the deprecated symbols
anyway.
</pre>
</div>
</content>
</entry>
<entry>
<title>buf: add consume_bytes</title>
<updated>2020-01-24T15:54:29+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2019-12-06T07:22:58+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=d68f2b1ab7aa7358f8ff0b75b72054bb9eeced3d'/>
<id>d68f2b1ab7aa7358f8ff0b75b72054bb9eeced3d</id>
<content type='text'>
Allow users to consume a buffer by the number of bytes, not just to an
ending pointer.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Allow users to consume a buffer by the number of bytes, not just to an
ending pointer.
</pre>
</div>
</content>
</entry>
<entry>
<title>buffer: fix printing into out-of-memory buffer</title>
<updated>2019-09-21T13:20:28+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-09-19T10:24:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=174b7a32d2ab480e4f3547758cf6c5ed30bbf5f4'/>
<id>174b7a32d2ab480e4f3547758cf6c5ed30bbf5f4</id>
<content type='text'>
Before printing into a `git_buf` structure, we always call `ENSURE_SIZE`
first. This macro will reallocate the buffer as-needed depending on
whether the current amount of allocated bytes is sufficient or not. If
`asize` is big enough, then it will just do nothing, otherwise it will
call out to `git_buf_try_grow`. But in fact, it is insufficient to only
check `asize`.

When we fail to allocate any more bytes e.g. via `git_buf_try_grow`,
then we set the buffer's pointer to `git_buf__oom`. Note that we touch
neither `asize` nor `size`. So if we just check `asize &gt; targetsize`,
then we will happily let the caller of `ENSURE_SIZE` proceed with an
out-of-memory buffer. As a result, we will print all bytes into the
out-of-memory buffer instead, resulting in an out-of-bounds write.

Fix the issue by having `ENSURE_SIZE` verify that the buffer is not
marked as OOM. Add a test to verify that we're not writing into the OOM
buffer.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Before printing into a `git_buf` structure, we always call `ENSURE_SIZE`
first. This macro will reallocate the buffer as-needed depending on
whether the current amount of allocated bytes is sufficient or not. If
`asize` is big enough, then it will just do nothing, otherwise it will
call out to `git_buf_try_grow`. But in fact, it is insufficient to only
check `asize`.

When we fail to allocate any more bytes e.g. via `git_buf_try_grow`,
then we set the buffer's pointer to `git_buf__oom`. Note that we touch
neither `asize` nor `size`. So if we just check `asize &gt; targetsize`,
then we will happily let the caller of `ENSURE_SIZE` proceed with an
out-of-memory buffer. As a result, we will print all bytes into the
out-of-memory buffer instead, resulting in an out-of-bounds write.

Fix the issue by having `ENSURE_SIZE` verify that the buffer is not
marked as OOM. Add a test to verify that we're not writing into the OOM
buffer.
</pre>
</div>
</content>
</entry>
<entry>
<title>buffer: fix infinite loop when growing buffers</title>
<updated>2019-09-21T13:20:28+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-09-19T10:46:37+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=208f1d7a4d9603ec6781ed18c10b240c7c5d2e33'/>
<id>208f1d7a4d9603ec6781ed18c10b240c7c5d2e33</id>
<content type='text'>
When growing buffers, we repeatedly multiply the currently allocated
number of bytes by 1.5 until it exceeds the requested number of bytes.
This has two major problems:

    1. If the current number of bytes is tiny and one wishes to resize
       to a comparatively huge number of bytes, then we may need to loop
       thousands of times.

    2. If resizing to a value close to `SIZE_MAX` (which would fail
       anyway), then we probably hit an infinite loop as multiplying the
       current amount of bytes will repeatedly result in integer
       overflows.

When reallocating buffers, one typically chooses values close to 1.5 to
enable re-use of resulting memory holes in later reallocations. But
because of this, it really only makes sense to use a factor of 1.5
_once_, but not looping until we finally are able to fit it. Thus, we
can completely avoid the loop and just opt for the much simpler
algorithm of multiplying with 1.5 once and, if the result doesn't fit,
just use the target size. This avoids both problems of looping
extensively and hitting overflows.

This commit also adds a test that would've previously resulted in an
infinite loop.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When growing buffers, we repeatedly multiply the currently allocated
number of bytes by 1.5 until it exceeds the requested number of bytes.
This has two major problems:

    1. If the current number of bytes is tiny and one wishes to resize
       to a comparatively huge number of bytes, then we may need to loop
       thousands of times.

    2. If resizing to a value close to `SIZE_MAX` (which would fail
       anyway), then we probably hit an infinite loop as multiplying the
       current amount of bytes will repeatedly result in integer
       overflows.

When reallocating buffers, one typically chooses values close to 1.5 to
enable re-use of resulting memory holes in later reallocations. But
because of this, it really only makes sense to use a factor of 1.5
_once_, but not looping until we finally are able to fit it. Thus, we
can completely avoid the loop and just opt for the much simpler
algorithm of multiplying with 1.5 once and, if the result doesn't fit,
just use the target size. This avoids both problems of looping
extensively and hitting overflows.

This commit also adds a test that would've previously resulted in an
infinite loop.
</pre>
</div>
</content>
</entry>
<entry>
<title>buffer: fix memory leak if unable to grow buffer</title>
<updated>2019-09-21T13:20:28+00:00</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2019-09-21T13:18:42+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=3e8a17b01b30a5aca5e10ef6f02635111315724e'/>
<id>3e8a17b01b30a5aca5e10ef6f02635111315724e</id>
<content type='text'>
If growing a buffer fails, we set its pointer to the static
`git_buf__oom` structure. While we correctly free the old pointer if
`git__malloc` returned an error, we do not free it if there was an
integer overflow while calculating the new allocation size. Fix this
issue by freeing the pointer to plug the memory leak.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
If growing a buffer fails, we set its pointer to the static
`git_buf__oom` structure. While we correctly free the old pointer if
`git__malloc` returned an error, we do not free it if there was an
integer overflow while calculating the new allocation size. Fix this
issue by freeing the pointer to plug the memory leak.
</pre>
</div>
</content>
</entry>
<entry>
<title>buffer: explicitly cast</title>
<updated>2019-01-25T22:47:39+00:00</updated>
<author>
<name>Edward Thomson</name>
<email>ethomson@edwardthomson.com</email>
</author>
<published>2019-01-21T00:57:39+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/libgit2.git/commit/?id=e5e2fac82116fe83cea3649d886af89e68ba5378'/>
<id>e5e2fac82116fe83cea3649d886af89e68ba5378</id>
<content type='text'>
Quiet down a warning from MSVC about how we're potentially losing data.
This is safe since we've explicitly tested it.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Quiet down a warning from MSVC about how we're potentially losing data.
This is safe since we've explicitly tested it.
</pre>
</div>
</content>
</entry>
</feed>
