<div><div>> Лучше подготовить патч поверх [2], и заодно</div><div>> убрать проверку из gfs2_log_release()<br /><br />Проверку убрал, делаю поверх ветки gfs2. Тогда</div><div>добавить ли второй Fixes с коммитом, который</div><div>проверку добавляет?</div></div><blockquote><p>On Tue, 14. Apr 01:34, Nikolai Kuznetsov wrote:</p><blockquote> Previous commit added an explicit NULL check for sd_jdesc to avoid</blockquote><p><br />При чтении не ясно, что за предыдущий коммит. Лучше написать явно<br /><br />Commit 35264909e9d1 ("gfs2: Fix NULL pointer dereference in<br />gfs2_log_flush") added ...<br /> </p><blockquote> a race with unmount. However, in gfs2_log_flush() already goto out<br /> when SDF_JOURNAL_LIVE is cleared, which happens during the final<br /> shutdown flush before sd_jdesc is set to NULL. The explicit check<br /> is redundant, remove it.</blockquote><p><br />Формулировка "in gfs2_log_flush() already goto out when SDF_JOURNAL_LIVE<br />is cleared" не соотносится с грамматикой англ. языка. Можно написать:<br />gfs2_log_flush() already checks for SDF_JOURNAL_LIVE bit and bails out if<br />it's not set. During unmount this bit is cleared in ... and the<br />filesystem becomes RO firstly and only then sd_jdesc is assigned NULL in<br />... So the race is not possible.<br /><br />Помимо того, что проверка redundant, в описании стоит явно упомянуть, что<br />указатель может быть разыменован дальше в функции gfs2_log_flush()<br />например там-то и там-то. Собственно на это и ругается стат. анализатор.<br />Иначе непонятно, зачем предлагаете убирать проверку. Ну есть она и есть<br />- зачем её убирать патчем? Сейчас описание довольно скупо.<br /><br />Также вы можете упомянуть в описании, что проблема, адресуемая виновным<br />коммитом, не является действительной по таким-то причинам. Это ведь так?<br /> </p><blockquote> <br /> Found by Linux Verification Center (linuxtesting.org) with SVACE.<br /> <br /> Fixes: 35264909e9d1 ("gfs2: Fix NULL pointer dereference in gfs2_log_flush")<br /> Cc: <a href="mailto:stable@vger.kernel.org" rel="noopener noreferrer">stable@vger.kernel.org</a></blockquote><p><br />Строчку Cc:stable нужно убрать. Патч удаляет лишнюю проверку - это<br />улучшение читаемости, логики, последовательности кода и т.д. и т.п., но не<br />фикс реальной ошибки или падения.<br /><br />Fixes можно оставить, это на усмотрение сопровождающего драйвера в данном<br />случае.<br /> </p><blockquote> Signed-off-by: Nikolai Kuznetsov <<a href="mailto:niku.csmsu@yandex.ru" rel="noopener noreferrer">niku.csmsu@yandex.ru</a>><br /> ---</blockquote><p><br />Ещё, т.к. патч готовится под отправку в апстрим межд. сообщества, его<br />стоит готовить поверх [1]. В идеале поверх [2]. Кстати, посмотрите на<br />последний коммит [3]. Это ветка сопровождающего файловой системы, в<br />которой он накапливает изменения для последующей отправки в апстрим.<br />Похоже там на эту ситуацию всё ещё смотрят как на проблемную.<br /><br />Своим патчем и, главное, доводами из описания патча (поэтому его важно<br />правильно составить), спровоцируете повторный анализ проблемы на его<br />стороне. Он либо согласится с нашей логикой, либо укажет на то, что<br />упускаем.<br /><br />Лучше подготовить патч поверх [2], и заодно убрать проверку из<br />gfs2_log_release(). Просьба его направить также пока только на<br />lvc-project.<br /><br />Чтобы не клонить к себе репозиторий с ядром ещё раз, можете провернуть<br />следующее изнутри того, который себе уже склонировали ('-C $CLONE_DIR'<br />можно опустить если уже находитесь внутри корня репозитория):<br /><br />git -C $CLONE_DIR remote add gfs2-origin <a href="https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/" rel="noopener noreferrer">https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/</a><br />git -C $CLONE_DIR fetch gfs2-origin<br />git -C $CLONE_DIR checkout --track gfs2-origin/for-next<br /><br />[1]: <a href="https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/" rel="noopener noreferrer">https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/</a><br />[2]: <a href="https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/log/?h=for-next" rel="noopener noreferrer">https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/log/?h=for-next</a><br />[3]: <a href="https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next&id=74b4dbb946060a3233604d91859a9abd3708141d" rel="noopener noreferrer">https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next&id=74b4dbb946060a3233604d91859a9abd3708141d</a><br /> </p><blockquote>  fs/gfs2/log.c | 5 +++--<br />  1 file changed, 3 insertions(+), 2 deletions(-)<br /> <br /> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c<br /> index 347df29d610e..e4457fe6d9d6 100644<br /> --- a/fs/gfs2/log.c<br /> +++ b/fs/gfs2/log.c<br /> @@ -1096,8 +1096,9 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)<br />          lops_before_commit(sdp, tr);<br />          if (gfs2_withdrawn(sdp))<br />                  goto out_withdraw;<br /> - if (sdp->sd_jdesc)<br /> - gfs2_log_submit_write(&sdp->sd_jdesc->jd_log_bio);<br /> +<br /> + gfs2_log_submit_write(&sdp->sd_jdesc->jd_log_bio);<br /> +<br />          if (gfs2_withdrawn(sdp))<br />                  goto out_withdraw;<br />  <br /> --<br /> 2.43.0</blockquote></blockquote>