Как избежать разыменования нулевого указателя, на примере одного исправления в ядре Linux

    Идея в следующем. Чтобы не было разыменования нулевого указателя, нужно, чтобы не было нулевого указателя. Ваш КО. Так сложилось, что однажды я исправил небольшую проблему в ядре Linux, но это была не текущая ветка ядра, а стабильная. В текущей, на тот момент, эту проблему тоже исправили, но по другому.

    Проблема была простая и весьма распространенная. Один поток освобождает буфер, во время, пока второй продолжает его использовать. Состояние гонки. Как избежать этой ситуации? Можно заставить один поток подождать, пока второй использует буфер, решение простое, но не эффективное, порой громоздкое.

    Первый вариант

    Мое решение было другим, а зачем, собственно ждать, пока освободиться буфер, чтобы тут же его стереть? Давайте оставим его в покое, буфер небольшой 512 байт погоды не сделает. Удалим все другие буферы, которые точно в данный момент не используются, а последний оставим. Это намного облегчит процесс синхронизации двух потоков ядра, об этом просто не надо будет думать. А данные, которые останутся в буфере, в любом случае могут быть, т.к. поступают туда по прерыванию, и даже, если бы мы освободили последний буфер, в тот же момент, по прерыванию, возможно выделение нового буфера и запись туда новых данных. Таким образом, получаем эффективное и более простое решение, чем решение в лоб. Пришлось удалять первое исправление и добавлять свое, что сделало исправление намного больше, чем должно было быть.

    Второй вариант
    Первоначальный вариант.
    ---
    diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
    index 6c9b7cd..4f02f9c 100644
    --- a/drivers/tty/tty_buffer.c
    +++ b/drivers/tty/tty_buffer.c
    @@ -114,11 +114,14 @@ static void __tty_buffer_flush(struct tty_struct *tty)
     {
     	struct tty_buffer *thead;
     
    -	while ((thead = tty->buf.head) != NULL) {
    -		tty->buf.head = thead->next;
    -		tty_buffer_free(tty, thead);
    +	if (tty->buf.head == NULL)
    +		return;
    +	while ((thead = tty->buf.head->next) != NULL) {
    +		tty_buffer_free(tty, tty->buf.head);
    +		tty->buf.head = thead;
     	}
    -	tty->buf.tail = NULL;
    +	WARN_ON(tty->buf.head != tty->buf.tail);
    +	tty->buf.head->read = tty->buf.head->commit;
     }
     
     /**
    

    Чтобы убрать «if», хорошо бы первый буфер выделять при открытии файла устройства, а не когда появятся первые данные, но это, усложнило бы «patch».

    Мое мнение о разработке ядра Linux
    Хорошо латать дыры первым попавшимся методом или плохо? Мне кажется что плохо, сам процесс внесения изменений в ядро выглядит, как какая-то гонка, кто успел тот и внес изменения, а последствия, потом разберемся. Мне кажется, что таких исправлений в ядро попадает слишком много, что еще больше запутывает и так не простой код.
    Поделиться публикацией

    Комментарии 17

      +4
      Как мало текста, и как много ошибок. И нифига непонятен смысл из вашего описания своего подхода.
        0
        А что непонятно?
        1. Можно пытаться избегать доступа по указателю со значением NULL.
        2. По возможности избегать указателей со значением NULL. Например, заранее выделять память или как в статье, освобождать ее не полностью.
        Конечно, не всегда так получится, но если есть такая возможность, второй подход, по моему мнению, лучше.
        0
        Где продолжение?
        0
        Позвольте спросить, а как второй вариант решает проблему? По моему никак, просто увеличилось кол-во инструкций, что может устранить race condition в данном конкретном случае на конкретной платформе.

        Я когда-то собаку сьел на синхронизации данных находящихся в списках и понял, что:
        — Нужно либо лочить доступ к списку на все время обращения к нему;
        — Если же объем операций над данными в списке велик, то проще под локом скопировать нужный кусок во временный список, а общий ресурс разлочить. Это позволило значительно увеличить производительность в многоядерных системах, но работает это лишь в том случае, если ваш код выполняет какие-то эксклюзивные операции над данными, которые не повлияют на ход операций в другом потоке.
          0
          Тут проблема была в следующем:
          Это код, изначально, так называемого flip буфера, пишем в один, читаем из другого, причем одновременно, потом меняем их местами, lock ставится только на момент смены буфера, на очень короткое время. Затем, количество буферов увеличилось, код изменился, но принцип остался.
          что может устранить race condition
          А нет больше никакого race condition. Мы просто не удаляем последний буфер, в который может идти запись, только стираем накопленные там данные.

          0
          небольшой code style вопрос:
          struct tty_buffer *thead;
           
          if (tty->buf.head == NULL)
          	return;
          

          Создается возможно неиспользуемая переменная, разве это не плохо?
            0
            Если вы о том, почему переменная не объявлена после if? Так принято, все переменные должны быть объявлены до кода.
            0
            Если я правильно понял, список теперь просто сокращается до 1 блока, вместо того чтобы удалять его полностью и устанавливать указатель в NULL.
            Тогда непонятно, зачем проверка на NULL:
            if (tty->buf.head == NULL)
            	return;
            
              0
              В начале, список пуст и нужно проверить, позже это исправлено. Ссылка в комментарии выше.
              0
              Изначально код удалял все элементы списка по порядку с первого по последний (включительно).
              Ваш код стал удалять все элементы списка по порядку с первого до последнего (но последний не удаляется).

              Почему бы прямо так и не написать в коде?

              while(tty->buf.head != tty->buf.tail) {
                  thead = tty->buf.head;
                  tty->buf.head = thead->next;
                  tty_buffer_free(tty, thead);
              }
              

                0
                Еще одно отличие. В оригинальном коде во время вызова tty_buffer_free в tty->buf.head лежит ссылка на следующий элемент. А в вашем коде там лежит ссылка на удаляемый элемент.
                ИМХО — слишком вольные изменения.
                  0
                  Согласен, так было бы яснее, но лишняя проверка не повредит.
                  WARN_ON(buf->head != buf->tail);
                    0
                    Ага, представьте как будет выглядеть эта проверка после while:
                    while(buf->head != buf->tail) {
                    ...
                    }
                    WARN_ON(buf->head != buf->tail);
                    


                    Хочу сказать, что проверка стала не лишней, а бессмысленной.
                      0
                      А если в коде ошибка или сбой произошел и tty->buf.tail == NULL, вернемся к тому с чего начали.
                      Или после сбоя, ошибки всегда tty->buf.tail != tty->buf.head — зависнем под lock с запрещенными перерываниями.
                      Хотя я уже написал, что так было бы яснее и возможно лучше.
                        0
                        Там этого WARN_ON уже давно нет. Смотрите 47aa658.
                    +1
                    Ой бяда, английский в вашем commit message хромает. Ну ничего, сам такой :)
                    Теперь по сути.
                    Хорошо латать дыры первым попавшимся методом или плохо? Мне кажется что плохо, сам процесс внесения изменений в ядро выглядит, как какая-то гонка, кто успел тот и внес изменения, а последствия, потом разберемся. Мне кажется, что таких исправлений в ядро попадает слишком много, что еще больше запутывает и так не простой код.

                    Дело в том, что изменения вносятся в основном «тусовкой». Если вы — человек со стороны, а я так вижу по вашим 3 изменениям, то порог входа будет не быстр, а ошибки устранять надо. Ну, и ваш выбор подсистемы tty — одной из самых старейших, конечно, вносит свои проблемы. Кстати, помните, что именно эта подсистема держала kernel global lock в ядре и только относительно недавно была исправлена. Т.е. я хочу сказать, что код там очень древний и очень запутанный.

                    Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

                    Самое читаемое