[lvc-project] [PATCH] fs/hfsplus: Use a more secure copy method.

[Fedor Pchelkin]

On Mon, 25. May 15:44, Markov Gleb wrote:
> From: Gleb Markov <markov.gi at npc-ksb.ru>
> 
> When copying, an additional byte is allocated for the null
> terminator and the buffer will definitely not be overflowed,

Зачем тогда что-либо менять?  Целесообразно ли это делать, если код в
явном виде учитывает ограничения?

> however, replacing the outdated method with a safe one seems
> to be the correct action.
> 

Тогда все предупреждения вида PROC_USE.VULNERABLE нужно разметить как
Confirmed и готвоить на всё патчи.  Но что-то этого никто не делает...

> Fixes: 5e61473ea9f1 ("fs/hfsplus: move xattr_name allocation in hfsplus_setxattr()")
> Fixes: a3cef4cd6886 ("fs/hfsplus: move xattr_name allocation in hfsplus_getxattr()")

Если ошибки нет, Fixes не нужны.

> @@ -425,12 +425,14 @@ int hfsplus_setxattr(struct inode *inode, const char *name,
>  	char *xattr_name;
>  	int res;
> +	size_t total_size;
> 
> -	xattr_name = kmalloc(NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1,
> -		GFP_KERNEL);
> +	total_size = NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN + 1;
> +	xattr_name = kmalloc(total_size, GFP_KERNEL);
> +
>  	if (!xattr_name)
>  		return -ENOMEM;
> -	strcpy(xattr_name, prefix);
> -	strcpy(xattr_name + prefixlen, name);

Касаемо изначального предположения о невозможности переполнения, уверены,
что длины передаваемых сюда строк prefix и name суммарно ограничены
NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN ?  Это стоит отследить по
коду и обосновать, хотя бы кратко.  То что там в конце +1 добавляется,
это хорошо и видимо действительно для нуль-терминатора, но это особо не
играет роли если вдруг по какой-то причине длины строк превышают
заданное ограничение в NLS_MAX_CHARSET_SIZE * HFSPLUS_ATTR_MAX_STRLEN.

> +	strscpy(xattr_name, prefix, total_size);
> +	strscpy(xattr_name + prefixlen, name, total_size - prefixlen);
>  	res = __hfsplus_setxattr(inode, xattr_name, value, size, flags);
>  	kfree(xattr_name);
>  	return res;