Чем мой вариант хуже?

Question

[xverizex]

Изучаю код tinyproxy. и вот приводится такой код. почему он такой избыточный? или мой вариант избыточный? вот вариант разработчика.

#define SEGMENT_LEN (512)
#define MAXIMUM_BUFFER_LENGTH (128 * 1024)
ssize_t readline (int fd, char **whole_buffer)
{
        ssize_t whole_buffer_len;
        char buffer[SEGMENT_LEN];
        char *ptr;

        ssize_t ret;
        ssize_t diff;

        struct read_lines_s {
                char *data;
                size_t len;
                struct read_lines_s *next;
        };
        struct read_lines_s *first_line, *line_ptr;

        first_line =
            (struct read_lines_s *) safecalloc (sizeof (struct read_lines_s),
                                                1);
        if (!first_line)
                return -ENOMEM;

        line_ptr = first_line;

        whole_buffer_len = 0;
        for (;;) {
                ret = recv (fd, buffer, SEGMENT_LEN, MSG_PEEK);
                if (ret <= 0)
                        goto CLEANUP;

                ptr = (char *) memchr (buffer, '\n', ret);
                if (ptr)
                        diff = ptr - buffer + 1;
                else
                        diff = ret;

                whole_buffer_len += diff;

                /*
                 * Don't allow the buffer to grow without bound. If we
                 * get to more than MAXIMUM_BUFFER_LENGTH close.
                 */
                if (whole_buffer_len > MAXIMUM_BUFFER_LENGTH) {
                        ret = -ERANGE;
                        goto CLEANUP;
                }

                line_ptr->data = (char *) safemalloc (diff);
                if (!line_ptr->data) {
                        ret = -ENOMEM;
                        goto CLEANUP;
                }

                ret = recv (fd, line_ptr->data, diff, 0);
                if (ret == -1) {
                        goto CLEANUP;
                }

                line_ptr->len = diff;

                if (ptr) {
                        line_ptr->next = NULL;
                        break;
                }

                line_ptr->next =
                    (struct read_lines_s *)
                    safecalloc (sizeof (struct read_lines_s), 1);
                if (!line_ptr->next) {
                        ret = -ENOMEM;
                        goto CLEANUP;
                }
                line_ptr = line_ptr->next;
        }

        *whole_buffer = (char *) safemalloc (whole_buffer_len + 1);
        if (!*whole_buffer) {
                ret = -ENOMEM;
                goto CLEANUP;
        }

        *(*whole_buffer + whole_buffer_len) = '\0';

        whole_buffer_len = 0;
        line_ptr = first_line;
        while (line_ptr) {
                memcpy (*whole_buffer + whole_buffer_len, line_ptr->data,
                        line_ptr->len);
                whole_buffer_len += line_ptr->len;

                line_ptr = line_ptr->next;
        }

        ret = whole_buffer_len;

CLEANUP:
        do {
                line_ptr = first_line->next;
                if (first_line->data)
                        safefree (first_line->data);
                safefree (first_line);
                first_line = line_ptr;
        } while (first_line);

        return ret;
}

ну я уже понимаю чем избыточен мой вариант. тем что чем больше данных, тем более медленней выделяется новая память с realloc. вот мой вариант, он не полный, я только нужный кусок привел.

ssize_t readline ( int fd, char **whole_buffer ) {
        ssize_t whole_buffer_len = 0;
        ssize_t ret;
        ssize_t pos = 0;
        char buffer[SEGMENT_LEN];

        *whole_buffer = calloc ( 0, sizeof ( char * ) );

        for(;;) {
                ret = recv ( fd, buffer, SEGMENT_LEN, 0 );
                if ( ret <= 0 ) goto CLEANUP;
                whole_buffer_len += ret;
                *whole_buffer = realloc ( *whole_buffer, whole_buffer_len );
                strncpy ( *whole_buffer + pos, buffer, ret );
                pos += ret;

        }
        ...
}

наверное если размер будет мегабайт, то следующая порция данных будет искать новую область памяти на несколько байт больше.

Answer 1

[Владимир Дубровин]

Исходный код считывает строку (до \n) а ваш - просто читает данные в буффер пока другой конец не закроет сокет.

Сравните все что есть в том коде и в вашем, что там есть и в вашем нет - того и не хватает. Например, возможны непрогнозируемые результаты (вероятней всего раскрытие содержимого памяти, т.к. остается неиницилизированный фрагмент с куском случайных данных из кучи в буфере) при наличии ASCII(0) в воде, не обрабатываются ошибки realloc() (что гарантировано приводит к повреждению памяти контролируемыми данными по частично контролируемому адресу, что с высокой вероятностью ведет к RCE, тем более что возможен heap spraying), код приводит к DoS-условиям с исчерпанием памяти, не терминируется текстовая строка и т.д. и т.п., т.е. только ошибок гарантированно влияющих на безопасность в вашем фрагменте не меньше 3х.

P.S. справделивости ради, исходный код тоже не очень. Используется в 2 раза больше системных вызовов с копированием данных из ядра в память, чем фактически требуется, при этом размер буфера, в который производится чтение не соответствует размеру страницы, что дополнительно замедляет процесс. Чтобы этого избежать надо было добавлять буферизацию в юзерспейсе. Но в нем хотя бы явных проблем с безопасностью не видно.

Comments

[xverizex]

да, но отделение по строкам бесмыссленная операция, если он потом собирает это всё обратно в буфер и очищает эту структуру. хотя я думал над его кодом и не мог придумать оптимизированней код. потому что у него как бы процесс разделения, а потом получения всего размера и сборки этого в один буфер. а я понял код, только не весь. а почему он копирует только одну строку, а остальные пропускает? я как бы понимаю, функция называется readline, что типа прочитать строку, но остальные отбрасываются же.

[Владимир Дубровин]

Они не отбрасывается, они оставляются в системном буфере и могут быть получены при последующих обращениях к recv(). Ваш код просто не сработает, он будет ждать пока другой конец не закроет сокет и не сможет отправить клиенту ответ, например.

Answer 2

[bogolt]

судя по описанию tinyxproxy он умеет модифицировать http заголовки. Думаю именно для этого используется разбиение по переносам строки и сохранение всех этих строк в отдельный список. Ну и вся эта избыточность как раз для этого предназначена.