[lvc-project] [PATCH] gfs2: Fix NULL pointer dereference in gfs2_log_flush

[Fedor Pchelkin]

On Mon, 06. Apr 18:05, Николай Кузнецов wrote:
>    > В чём заключается ошибка?
>    В функцию gfs2_log_flush может прийти нулевой указатель
>    sdp->sd_jdesc, после чего будет цепочка вызовов:
>    gfs2_log_shutdown(sdp) -> log_pull_tail(sdp) ->
>    dist = log_distance(sdp, new_tail, sdp->sd_log_tail), а в log_distance
>    уже происходит его разыменование. Согласен, из патча это неясно, перепишу.
> 
>    > То есть после патча по неизвестной (пока) причине будет пропускаться
>    > код, который вроде как и готов обрабатывать нулевой указатель
>    Тот код был добавлен как раз в целях избежания разыменования
>    в конкретном месте, прикладываю ссылку на коммит:
>    [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/gfs2/log.c?id=35264909e9d1973ab9aaa2a1b07cda70f12bb828

Добавляя свою проверку в начало gfs2_log_flush(), нивелируете изменения
этого патча.  В нём автор пишет, что да, указатель действительно может
оказаться нулевым на этапе gfs2_log_flush() при таких-то условиях, поэтому
его нужно обработать.

>    Там нет полноценной обработки указателя, как я понял,
>    только обработка частной траектории выполнения.
> 
>    В данном коммите нет тега Fixes, я последовал примеру и не стал
>    его добавлять.
>    Действительно, я думал над тем, чтобы убрать обработку
>    указателя в том месте, но старался следовать инструкции с

Нет, проверка, добавленная в 35264909e9d1 ("gfs2: Fix NULL pointer
dereference in gfs2_log_flush") должна быть нужной.  Зачем её предлагаете
убирать?

>    Мне показалось логичнее сделать ассерт в начале,
>    чтобы в следующей строчке сразу было попадание на проверку
>    withdrawn.

Это не выглядит логичным, учитывая происходящее в коммите 35264909e9d1
("gfs2: Fix NULL pointer dereference in gfs2_log_flush").  Из него следует,
что указателю валидно быть нулевым - это не ошибочная ситуация на ассерт.

Как вы упомянули выше, Свейс подцепил один конкретный путь
gfs2_log_shutdown(sdp) -> log_pull_tail(sdp) -> log_distance().  Возможно
суть кроется в этом пути?