Skip to content

Commit

Permalink
journal-remote: use macro wrapper instead of alloca to extend string
Browse files Browse the repository at this point in the history
We would use alloca to extend the format string with "\n". We do this
automatically in order to not forget appending the newline everywhere.
We can simplify the whole thing by using a macro to append the newline instead,
which means that we don't need to copy the string.

Because we concatenate the string argument with another literal string, we know
it must a literal string. Thus it's not a problem that it is "evaluated" two
times.

Quoting Hristo Venev:
> Since commit f5e757f, mhd_respond() adds a
> newline to its argument before passing it on to mhd_respond_internal(). This
> is done via an alloca()-allocated buffer. However, MHD_RESPMEM_PERSISTENT is
> given as a flag to MHD_create_response_from_buffer(), leading to a
> use-after-free later when the response is sent. Replacing
> MHD_RESPMEM_PERSISTENT with MHD_RESPMEM_MUST_COPY appears to fix the issue.

MHD_RESPMEM_MUST_COPY would work, but we also use mhd_respond() for mhd_oom(),
and we don't want to allocate in an oom scenario in order to maximize the
possibility that an answer will be delivered. Using the macro magic makes this
nicer and we get rid of the code doing alloca.

Fixes an issue reported by Hristo Venev.
Fixes systemd/systemd#9858.
  • Loading branch information
keszybz committed Jan 15, 2024
1 parent 204594e commit 320ff93
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 35 deletions.
42 changes: 14 additions & 28 deletions src/journal-remote/microhttpd-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ void microhttpd_logger(void *arg, const char *fmt, va_list ap) {
REENABLE_WARNING;
}

static int mhd_respond_internal(struct MHD_Connection *connection,
enum MHD_RequestTerminationCode code,
const char *buffer,
size_t size,
enum MHD_ResponseMemoryMode mode) {
int mhd_respond_internal(
struct MHD_Connection *connection,
enum MHD_RequestTerminationCode code,
const char *buffer,
size_t size,
enum MHD_ResponseMemoryMode mode) {

assert(connection);

_cleanup_(MHD_destroy_responsep) struct MHD_Response *response
Expand All @@ -43,29 +45,16 @@ static int mhd_respond_internal(struct MHD_Connection *connection,
return MHD_queue_response(connection, code, response);
}

int mhd_respond(struct MHD_Connection *connection,
enum MHD_RequestTerminationCode code,
const char *message) {

const char *fmt;

fmt = strjoina(message, "\n");

return mhd_respond_internal(connection, code,
fmt, strlen(message) + 1,
MHD_RESPMEM_PERSISTENT);
}

int mhd_respond_oom(struct MHD_Connection *connection) {
return mhd_respond(connection, MHD_HTTP_SERVICE_UNAVAILABLE, "Out of memory.");
return mhd_respond(connection, MHD_HTTP_SERVICE_UNAVAILABLE, "Out of memory.");
}

int mhd_respondf(struct MHD_Connection *connection,
int error,
enum MHD_RequestTerminationCode code,
const char *format, ...) {
int mhd_respondf_internal(
struct MHD_Connection *connection,
int error,
enum MHD_RequestTerminationCode code,
const char *format, ...) {

const char *fmt;
char *m;
int r;
va_list ap;
Expand All @@ -76,11 +65,8 @@ int mhd_respondf(struct MHD_Connection *connection,
if (error < 0)
error = -error;
errno = -error;
fmt = strjoina(format, "\n");
va_start(ap, format);
DISABLE_WARNING_FORMAT_NONLITERAL;
r = vasprintf(&m, fmt, ap);
REENABLE_WARNING;
r = vasprintf(&m, format, ap);
va_end(ap);

if (r < 0)
Expand Down
31 changes: 24 additions & 7 deletions src/journal-remote/microhttpd-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,34 @@ void microhttpd_logger(void *arg, const char *fmt, va_list ap) _printf_(2, 0);
/* respond_oom() must be usable with return, hence this form. */
#define respond_oom(connection) log_oom(), mhd_respond_oom(connection)

int mhd_respondf(struct MHD_Connection *connection,
int error,
enum MHD_RequestTerminationCode code,
const char *format, ...) _printf_(4,5);

int mhd_respond(struct MHD_Connection *connection,
int mhd_respond_internal(
struct MHD_Connection *connection,
enum MHD_RequestTerminationCode code,
const char *message);
const char *buffer,
size_t size,
enum MHD_ResponseMemoryMode mode);

#define mhd_respond(connection, code, message) \
mhd_respond_internal( \
connection, code, \
message "\n", \
strlen(message) + 1, \
MHD_RESPMEM_PERSISTENT)

int mhd_respond_oom(struct MHD_Connection *connection);

int mhd_respondf_internal(
struct MHD_Connection *connection,
int error,
enum MHD_RequestTerminationCode code,
const char *format, ...) _printf_(4,5);

#define mhd_respondf(connection, error, code, format, ...) \
mhd_respondf_internal( \
connection, error, code, \
format "\n", \
##__VA_ARGS__)

int check_permissions(struct MHD_Connection *connection, int *code, char **hostname);

/* Set gnutls internal logging function to a callback which uses our
Expand Down

0 comments on commit 320ff93

Please sign in to comment.