[lvc-project] [PATCH v6.1] mm/damon: get rid of overlapping areas

[Марков Глеб Игоревич]

Среда, Июнь 24, 2026 14:48 MSK, Марков Глеб Игоревич <markov.gi at npc-ksb.ru> писал(а):

> Среда, Июнь 24, 2026 13:38 MSK, Fedor Pchelkin <pchelkin at ispras.ru> писал(а):
> 
> > Патч нужен и для 6.12 наверно, не только 6.1.
> > 
> > On Wed, 24. Jun 12:22, Markov Gleb wrote:
> > > Change sscanf() function to avoid possible overlap situation.
> > 
> > Наверно не change, а replace чем-то другим.  Ведь не меняете же
> > внутренности функции sscanf().
> > 
> > > 
> > > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > > 
> > > Fixes: 4bc05954d007 ("mm/damon: implement a debugfs-based user space interface")
> > > Signed-off-by: Gleb Markov <markov.gi at npc-ksb.ru>
> > > ---
> > >  mm/damon/dbgfs.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/mm/damon/dbgfs.c b/mm/damon/dbgfs.c
> > > index b3f454a5c682..62b1e7474445 100644
> > > --- a/mm/damon/dbgfs.c
> > > +++ b/mm/damon/dbgfs.c
> > > @@ -1003,17 +1003,20 @@ static ssize_t dbgfs_monitor_on_write(struct file *file,
> > >  {
> > >  	ssize_t ret;
> > >  	char *kbuf;
> > > +	char *sd_kbuf;
> > >  
> > >  	kbuf = user_input_str(buf, count, ppos);
> > >  	if (IS_ERR(kbuf))
> > >  		return PTR_ERR(kbuf);
> > >  
> > >  	/* Remove white space */
> > > -	if (sscanf(kbuf, "%s", kbuf) != 1) {
> > > +	sd_kbuf = strim(kbuf);
> > > +	if (*sd_kbuf == '\0') {
> > >  		kfree(kbuf);
> > >  		return -EINVAL;
> > >  	}
> > 
> > sd_kbuf теперь указывает на первый непробельный символ входной
> > буфер-строки.  Однако сам kbuf, с которым дальше в функции идёт работа,
> > продолжает указывать на начало исходного буфера с потенциально
> > пробельными символами в начале.
> > 
> > Если хотели убрать пробелы из начала, текущий патч этого не решает.
> > 
> > Резона в добавлении sd_kbuf не вижу.  В целом достаточно
> > 
> > 	kbuf = strim(kbuf);
> > 
> > >  
> > > +
> > 
> > Зачем-то добавленная лишняя пустая строка.
> > 
> > >  	mutex_lock(&damon_dbgfs_lock);
> > >  	if (!strncmp(kbuf, "on", count)) {
> > >  		int i;
> > > -- 
> > > 2.43.0
> 
> kbuf - указатель на выделенный буфер (кусок памяти). strim() сместит указатель при обрезке в выделенном
> буфере, при последующем вызове kfree() ожидается очистка того же выделенного блока памяти (начинающегося с того
> же адреса), но при вызове strim(), где в качестве источника и результата будет использоваться указатель на
> ранее выделенный kbuf, в результате отработки skip_spaces(), значение указателя изменится (по результатам
> смещения, в связи с чем в следующем снэпшоте появится срабатывание формата LEAK.
> В идеале работать с выделенным токеном (sd_kbuf) далее чтобы подобных кейсов не возникало.
> Все же хотел бы настоять на замене strncmp() на strcmp(), поскольку таким образом не будет явной
> привязки к передаваемому в метод `count`, а сравнение не изменится по логике.
> 
> Предположим, что помимо "on" или "off" будет передан "on " или "off ", что явно корректнее будет
> обрабатываться обычным strim().
> 
> В любом случае, исправления относительно работы с обрезанной строкой будут внесены и по итогам
> ответа на это сообщение будут отправлены повторно.

Если сформулировать проще, то strim() вызывает skip_spaces(), который сам указатель сместит относительно
ранее выделенного через kmalloc() блока. Попытка очистки путем вызова kfree() не отработает корректно, а 
значит требуется создание временной локальной переменной для сохранения оригинального значения
указателя.

Использование strcmp() вместо strncmp() предлагается лишь на случай использования нестрого инпута. 
Отсутствие привязки к передаваемому в метод `count` позволяет более строго сравнивать значения, что
по логике и является корректным поведением.