evbuffer_add_file: do not use evbuffer_remove(), instead calc offset for mmap()

@ellzey:
   "when evbuffer_add_file is called and mmap is used, if the offset
    argument is >0, a mistake happens: add_file removes "offset" byts
    from the front of the evbuffer.
    So that means any data that was previously on the buffer is trimmed
    off by "offset" bytes. Whoops."

And even more when offset>0, evbuffer_add_file() still maps whole file,
let's calc appropriate offset and length.

And here is the wrong fix:
  afca576a05
This commit is contained in:
Azat Khuzhin 2017-12-18 02:02:15 +03:00
parent 765c35d4a8
commit 2653be970a

View File

@ -2769,6 +2769,20 @@ done:
return result; return result;
} }
#ifdef _EVENT_HAVE_MMAP
static long
get_page_size(void)
{
#ifdef SC_PAGE_SIZE
return sysconf(SC_PAGE_SIZE);
#elif defined(_SC_PAGE_SIZE)
return sysconf(_SC_PAGE_SIZE);
#else
return 1;
#endif
}
#endif
/* TODO(niels): maybe we don't want to own the fd, however, in that /* TODO(niels): maybe we don't want to own the fd, however, in that
* case, we should dup it - dup is cheap. Perhaps, we should use a * case, we should dup it - dup is cheap. Perhaps, we should use a
* callback instead? * callback instead?
@ -2829,7 +2843,20 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd,
#endif #endif
#if defined(_EVENT_HAVE_MMAP) #if defined(_EVENT_HAVE_MMAP)
if (use_mmap) { if (use_mmap) {
void *mapped = mmap(NULL, length + offset, PROT_READ, off_t offset_rounded = 0, offset_leftover = 0;
void *mapped;
if (offset) {
/* mmap implementations don't generally like us
* to have an offset that isn't a round */
long page_size = get_page_size();
if (page_size == -1) {
event_warn("%s: cannot detect PAGE_SIZE", __func__);
return (-1);
}
offset_leftover = offset % page_size;
offset_rounded = offset - offset_leftover;
}
mapped = mmap(NULL, length + offset_leftover, PROT_READ,
#ifdef MAP_NOCACHE #ifdef MAP_NOCACHE
MAP_NOCACHE | MAP_NOCACHE |
#endif #endif
@ -2837,13 +2864,7 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd,
MAP_FILE | MAP_FILE |
#endif #endif
MAP_PRIVATE, MAP_PRIVATE,
fd, 0); fd, offset_rounded);
/* some mmap implementations require offset to be a multiple of
* the page size. most users of this api, are likely to use 0
* so mapping everything is not likely to be a problem.
* TODO(niels): determine page size and round offset to that
* page size to avoid mapping too much memory.
*/
if (mapped == MAP_FAILED) { if (mapped == MAP_FAILED) {
event_warn("%s: mmap(%d, %d, %zu) failed", event_warn("%s: mmap(%d, %d, %zu) failed",
__func__, fd, 0, (size_t)(length + offset)); __func__, fd, 0, (size_t)(length + offset));
@ -2852,14 +2873,15 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd,
chain = evbuffer_chain_new(sizeof(struct evbuffer_chain_fd)); chain = evbuffer_chain_new(sizeof(struct evbuffer_chain_fd));
if (chain == NULL) { if (chain == NULL) {
event_warn("%s: out of memory", __func__); event_warn("%s: out of memory", __func__);
munmap(mapped, length + offset); munmap(mapped, length + offset_leftover);
return (-1); return (-1);
} }
chain->flags |= EVBUFFER_MMAP | EVBUFFER_IMMUTABLE; chain->flags |= EVBUFFER_MMAP | EVBUFFER_IMMUTABLE;
chain->buffer = mapped; chain->buffer = mapped;
chain->buffer_len = length + offset; chain->buffer_len = length + offset_leftover;
chain->off = length + offset; chain->off = length;
chain->misalign = offset_leftover;
info = EVBUFFER_CHAIN_EXTRA(struct evbuffer_chain_fd, chain); info = EVBUFFER_CHAIN_EXTRA(struct evbuffer_chain_fd, chain);
info->fd = fd; info->fd = fd;
@ -2871,11 +2893,7 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd,
ok = 0; ok = 0;
} else { } else {
outbuf->n_add_for_cb += length; outbuf->n_add_for_cb += length;
evbuffer_chain_insert(outbuf, chain); evbuffer_chain_insert(outbuf, chain);
/* we need to subtract whatever we don't need */
evbuffer_drain(outbuf, offset);
} }
} else } else
#endif #endif