mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-02-24 22:56:55 +00:00
[BUG] http: fix content-length handling on 32-bit platforms
Despite much care around handling the content-length as a 64-bit integer, forwarding was broken on 32-bit platforms due to the 32-bit nature of the ->to_forward member of the "buffer" struct. The issue is that this member is declared as a long, so while it works OK on 64-bit platforms, 32-bit truncate the content-length to the lower 32-bits. One solution could consist in turning to_forward to a long long, but it is used a lot in the critical path, so it's not acceptable to perform all buffer size computations on 64-bit there. The fix consists in changing the to_forward member to a strict 32-bit integer and ensure in buffer_forward() that only the amount of bytes that can fit into it is considered. Callers of buffer_forward() are responsible for checking that their data were taken into account. We arbitrarily ensure we never consider more than 2G at once. That's the way it was intended to work on 32-bit platforms except that it did not. This issue was tracked down hard at Exosec with Bertrand Jacquin, Thierry Fournier and Julien Thomas. It remained undetected for a long time because files larger than 4G are almost always transferred in chunked-encoded format, and most platforms dealing with huge contents these days run on 64-bit. The bug affects all 1.5 and 1.4 versions, and must be backported.
This commit is contained in:
parent
d3db94399f
commit
d8ee85a0a3
@ -110,40 +110,61 @@ static inline void buffer_check_timeouts(struct buffer *b)
|
||||
b->flags |= BF_ANA_TIMEOUT;
|
||||
}
|
||||
|
||||
/* Schedule <bytes> more bytes to be forwarded by the buffer without notifying
|
||||
/* Schedule up to <bytes> more bytes to be forwarded by the buffer without notifying
|
||||
* the task. Any pending data in the buffer is scheduled to be sent as well,
|
||||
* in the limit of the number of bytes to forward. This must be the only method
|
||||
* to use to schedule bytes to be sent. Directly touching ->to_forward will
|
||||
* cause lockups when send_max goes down to zero if nobody is ready to push the
|
||||
* remaining data.
|
||||
* to use to schedule bytes to be sent. If the requested number is too large, it
|
||||
* is automatically adjusted. The number of bytes taken into account is returned.
|
||||
* Directly touching ->to_forward will cause lockups when send_max goes down to
|
||||
* zero if nobody is ready to push the remaining data.
|
||||
*/
|
||||
static inline void buffer_forward(struct buffer *buf, unsigned long bytes)
|
||||
static inline unsigned long long buffer_forward(struct buffer *buf, unsigned long long bytes)
|
||||
{
|
||||
unsigned long data_left;
|
||||
unsigned int data_left;
|
||||
unsigned int new_forward;
|
||||
|
||||
if (!bytes)
|
||||
return;
|
||||
return 0;
|
||||
data_left = buf->l - buf->send_max;
|
||||
if (data_left >= bytes) {
|
||||
if (bytes <= (unsigned long long)data_left) {
|
||||
buf->send_max += bytes;
|
||||
buf->flags &= ~BF_OUT_EMPTY;
|
||||
return;
|
||||
return bytes;
|
||||
}
|
||||
|
||||
buf->send_max += data_left;
|
||||
if (buf->send_max)
|
||||
buf->flags &= ~BF_OUT_EMPTY;
|
||||
|
||||
if (buf->to_forward != BUF_INFINITE_FORWARD) {
|
||||
buf->to_forward += bytes - data_left;
|
||||
if (bytes == BUF_INFINITE_FORWARD)
|
||||
buf->to_forward = bytes;
|
||||
}
|
||||
|
||||
if (buf->l < buffer_max_len(buf))
|
||||
buf->flags &= ~BF_FULL;
|
||||
else
|
||||
buf->flags |= BF_FULL;
|
||||
|
||||
if (likely(bytes == BUF_INFINITE_FORWARD)) {
|
||||
buf->to_forward = bytes;
|
||||
return bytes;
|
||||
}
|
||||
|
||||
/* Note: the case below is the only case where we may return
|
||||
* a byte count that does not fit into a 32-bit number.
|
||||
*/
|
||||
if (likely(buf->to_forward == BUF_INFINITE_FORWARD))
|
||||
return bytes;
|
||||
|
||||
new_forward = buf->to_forward + bytes - data_left;
|
||||
bytes = data_left; /* at least those bytes were scheduled */
|
||||
|
||||
if (new_forward <= buf->to_forward) {
|
||||
/* integer overflow detected, let's assume no more than 2G at once */
|
||||
new_forward = MID_RANGE(new_forward);
|
||||
}
|
||||
|
||||
if (new_forward > buf->to_forward) {
|
||||
bytes += new_forward - buf->to_forward;
|
||||
buf->to_forward = new_forward;
|
||||
}
|
||||
return bytes;
|
||||
}
|
||||
|
||||
/* Schedule all remaining buffer data to be sent. send_max is not touched if it
|
||||
|
@ -160,7 +160,7 @@
|
||||
|
||||
|
||||
/* Magic value to forward infinite size (TCP, ...), used with ->to_forward */
|
||||
#define BUF_INFINITE_FORWARD (~0UL)
|
||||
#define BUF_INFINITE_FORWARD MAX_RANGE(int)
|
||||
|
||||
/* describes a chunk of string */
|
||||
struct chunk {
|
||||
@ -182,7 +182,7 @@ struct buffer {
|
||||
char *r, *w, *lr; /* read ptr, write ptr, last read */
|
||||
unsigned int size; /* buffer size in bytes */
|
||||
unsigned int send_max; /* number of bytes the sender can consume om this buffer, <= l */
|
||||
unsigned long to_forward; /* number of bytes to forward after send_max without a wake-up */
|
||||
unsigned int to_forward; /* number of bytes to forward after send_max without a wake-up */
|
||||
unsigned int analysers; /* bit field indicating what to do on the buffer */
|
||||
int analyse_exp; /* expiration date for current analysers (if set) */
|
||||
void (*hijacker)(struct session *, struct buffer *); /* alternative content producer */
|
||||
|
@ -2815,7 +2815,7 @@ int stats_dump_full_sess_to_buffer(struct stream_interface *si)
|
||||
|
||||
|
||||
chunk_printf(&msg,
|
||||
" req=%p (f=0x%06x an=0x%x l=%d sndmx=%d pipe=%d fwd=%ld)\n"
|
||||
" req=%p (f=0x%06x an=0x%x l=%d sndmx=%d pipe=%d fwd=%d)\n"
|
||||
" an_exp=%s",
|
||||
sess->req,
|
||||
sess->req->flags, sess->req->analysers,
|
||||
@ -2845,7 +2845,7 @@ int stats_dump_full_sess_to_buffer(struct stream_interface *si)
|
||||
sess->req->total);
|
||||
|
||||
chunk_printf(&msg,
|
||||
" res=%p (f=0x%06x an=0x%x l=%d sndmx=%d pipe=%d fwd=%ld)\n"
|
||||
" res=%p (f=0x%06x an=0x%x l=%d sndmx=%d pipe=%d fwd=%d)\n"
|
||||
" an_exp=%s",
|
||||
sess->rep,
|
||||
sess->rep->flags, sess->rep->analysers,
|
||||
|
@ -4410,15 +4410,17 @@ int http_request_forward_body(struct session *s, struct buffer *req, int an_bit)
|
||||
}
|
||||
|
||||
while (1) {
|
||||
int bytes;
|
||||
|
||||
http_silent_debug(__LINE__, s);
|
||||
/* we may have some data pending */
|
||||
if (msg->chunk_len || msg->som != msg->sov) {
|
||||
int bytes = msg->sov - msg->som;
|
||||
if (bytes < 0) /* sov may have wrapped at the end */
|
||||
bytes += req->size;
|
||||
buffer_forward(req, bytes + msg->chunk_len);
|
||||
msg->chunk_len = 0; /* don't forward that again */
|
||||
bytes = msg->sov - msg->som;
|
||||
if (msg->chunk_len || bytes) {
|
||||
msg->som = msg->sov;
|
||||
if (likely(bytes < 0)) /* sov may have wrapped at the end */
|
||||
bytes += req->size;
|
||||
msg->chunk_len += (unsigned int)bytes;
|
||||
msg->chunk_len -= buffer_forward(req, msg->chunk_len);
|
||||
}
|
||||
|
||||
if (msg->msg_state == HTTP_MSG_DATA) {
|
||||
@ -5421,6 +5423,7 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit
|
||||
{
|
||||
struct http_txn *txn = &s->txn;
|
||||
struct http_msg *msg = &s->txn.rsp;
|
||||
int bytes;
|
||||
|
||||
if (unlikely(msg->msg_state < HTTP_MSG_BODY))
|
||||
return 0;
|
||||
@ -5454,17 +5457,20 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit
|
||||
}
|
||||
|
||||
while (1) {
|
||||
int bytes;
|
||||
|
||||
http_silent_debug(__LINE__, s);
|
||||
/* we may have some data pending */
|
||||
if (msg->chunk_len || msg->som != msg->sov) {
|
||||
int bytes = msg->sov - msg->som;
|
||||
if (bytes < 0) /* sov may have wrapped at the end */
|
||||
bytes += res->size;
|
||||
buffer_forward(res, bytes + msg->chunk_len);
|
||||
msg->chunk_len = 0; /* don't forward that again */
|
||||
bytes = msg->sov - msg->som;
|
||||
if (msg->chunk_len || bytes) {
|
||||
msg->som = msg->sov;
|
||||
if (likely(bytes < 0)) /* sov may have wrapped at the end */
|
||||
bytes += res->size;
|
||||
msg->chunk_len += (unsigned int)bytes;
|
||||
msg->chunk_len -= buffer_forward(res, msg->chunk_len);
|
||||
}
|
||||
|
||||
|
||||
if (msg->msg_state == HTTP_MSG_DATA) {
|
||||
/* must still forward */
|
||||
if (res->to_forward)
|
||||
@ -5573,14 +5579,14 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit
|
||||
if (!s->req->analysers)
|
||||
goto return_bad_res;
|
||||
|
||||
/* forward the chunk size as well as any pending data */
|
||||
if (msg->chunk_len || msg->som != msg->sov) {
|
||||
int bytes = msg->sov - msg->som;
|
||||
if (bytes < 0) /* sov may have wrapped at the end */
|
||||
bytes += res->size;
|
||||
buffer_forward(res, bytes + msg->chunk_len);
|
||||
msg->chunk_len = 0; /* don't forward that again */
|
||||
/* forward any pending data */
|
||||
bytes = msg->sov - msg->som;
|
||||
if (msg->chunk_len || bytes) {
|
||||
msg->som = msg->sov;
|
||||
if (likely(bytes < 0)) /* sov may have wrapped at the end */
|
||||
bytes += res->size;
|
||||
msg->chunk_len += (unsigned int)bytes;
|
||||
msg->chunk_len -= buffer_forward(res, msg->chunk_len);
|
||||
}
|
||||
|
||||
/* When TE: chunked is used, we need to get there again to parse remaining
|
||||
|
Loading…
Reference in New Issue
Block a user