Skip to content

Commit 3253db0

Browse files
keszybzbluca
authored andcommitted
journal-remote: use macro wrapper instead of alloca to extend string
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. (cherry picked from commit 320ff93) (cherry picked from commit fb974c8)
1 parent b4ef58c commit 3253db0

File tree

2 files changed

+38
-35
lines changed

2 files changed

+38
-35
lines changed

src/journal-remote/microhttpd-util.c

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,13 @@ void microhttpd_logger(void *arg, const char *fmt, va_list ap) {
2525
REENABLE_WARNING;
2626
}
2727

28-
static int mhd_respond_internal(struct MHD_Connection *connection,
29-
enum MHD_RequestTerminationCode code,
30-
const char *buffer,
31-
size_t size,
32-
enum MHD_ResponseMemoryMode mode) {
28+
int mhd_respond_internal(
29+
struct MHD_Connection *connection,
30+
enum MHD_RequestTerminationCode code,
31+
const char *buffer,
32+
size_t size,
33+
enum MHD_ResponseMemoryMode mode) {
34+
3335
assert(connection);
3436

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

46-
int mhd_respond(struct MHD_Connection *connection,
47-
enum MHD_RequestTerminationCode code,
48-
const char *message) {
49-
50-
const char *fmt;
51-
52-
fmt = strjoina(message, "\n");
53-
54-
return mhd_respond_internal(connection, code,
55-
fmt, strlen(message) + 1,
56-
MHD_RESPMEM_PERSISTENT);
57-
}
58-
5948
int mhd_respond_oom(struct MHD_Connection *connection) {
60-
return mhd_respond(connection, MHD_HTTP_SERVICE_UNAVAILABLE, "Out of memory.");
49+
return mhd_respond(connection, MHD_HTTP_SERVICE_UNAVAILABLE, "Out of memory.");
6150
}
6251

63-
int mhd_respondf(struct MHD_Connection *connection,
64-
int error,
65-
enum MHD_RequestTerminationCode code,
66-
const char *format, ...) {
52+
int mhd_respondf_internal(
53+
struct MHD_Connection *connection,
54+
int error,
55+
enum MHD_RequestTerminationCode code,
56+
const char *format, ...) {
6757

68-
const char *fmt;
6958
char *m;
7059
int r;
7160
va_list ap;
@@ -76,11 +65,8 @@ int mhd_respondf(struct MHD_Connection *connection,
7665
if (error < 0)
7766
error = -error;
7867
errno = -error;
79-
fmt = strjoina(format, "\n");
8068
va_start(ap, format);
81-
DISABLE_WARNING_FORMAT_NONLITERAL;
82-
r = vasprintf(&m, fmt, ap);
83-
REENABLE_WARNING;
69+
r = vasprintf(&m, format, ap);
8470
va_end(ap);
8571

8672
if (r < 0)

src/journal-remote/microhttpd-util.h

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,34 @@ void microhttpd_logger(void *arg, const char *fmt, va_list ap) _printf_(2, 0);
6262
/* respond_oom() must be usable with return, hence this form. */
6363
#define respond_oom(connection) log_oom(), mhd_respond_oom(connection)
6464

65-
int mhd_respondf(struct MHD_Connection *connection,
66-
int error,
67-
enum MHD_RequestTerminationCode code,
68-
const char *format, ...) _printf_(4,5);
69-
70-
int mhd_respond(struct MHD_Connection *connection,
65+
int mhd_respond_internal(
66+
struct MHD_Connection *connection,
7167
enum MHD_RequestTerminationCode code,
72-
const char *message);
68+
const char *buffer,
69+
size_t size,
70+
enum MHD_ResponseMemoryMode mode);
71+
72+
#define mhd_respond(connection, code, message) \
73+
mhd_respond_internal( \
74+
connection, code, \
75+
message "\n", \
76+
strlen(message) + 1, \
77+
MHD_RESPMEM_PERSISTENT)
7378

7479
int mhd_respond_oom(struct MHD_Connection *connection);
7580

81+
int mhd_respondf_internal(
82+
struct MHD_Connection *connection,
83+
int error,
84+
enum MHD_RequestTerminationCode code,
85+
const char *format, ...) _printf_(4,5);
86+
87+
#define mhd_respondf(connection, error, code, format, ...) \
88+
mhd_respondf_internal( \
89+
connection, error, code, \
90+
format "\n", \
91+
##__VA_ARGS__)
92+
7693
int check_permissions(struct MHD_Connection *connection, int *code, char **hostname);
7794

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

0 commit comments

Comments
 (0)