[PATCH] arm64: ptrace: user_regset_copyin_ignore() always returns 0

[Sergey Shtylyov]

Hello!

On 9/26/22 5:01 PM, Catalin Marinas wrote:

[...]
>>>> user_regset_copyin_ignore() always return 0, so checking its result seems
>>>> pointless -- don't do this...
>>>>
>>>> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
>>>> analysis tool.
>>>>
>>>> Signed-off-by: Sergey Shtylyov <s.shtylyov at omp.ru>
>>>>
>>>> ---
>>>> This patch is against the 'for-next/core' branch of the ARM64 repo...
>>>>
>>>>  arch/arm64/kernel/ptrace.c |   16 ++++------------
>>>>  1 file changed, 4 insertions(+), 12 deletions(-)
>>>>
>>>> Index: linux/arch/arm64/kernel/ptrace.c
>>>> ===================================================================
>>>> --- linux.orig/arch/arm64/kernel/ptrace.c
>>>> +++ linux/arch/arm64/kernel/ptrace.c
>>>> @@ -514,9 +514,7 @@ static int hw_break_set(struct task_stru
>>>>  
>>>>  	/* Resource info and pad */
>>>>  	offset = offsetof(struct user_hwdebug_state, dbg_regs);
>>>> -	ret = user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf, 0, offset);
>>>> -	if (ret)
>>>> -		return ret;
>>>> +	user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf, 0, offset);
>>>>  
>>>>  	/* (address, ctrl) registers */
>>>>  	limit = regset->n * regset->size;
>>>> @@ -543,11 +541,8 @@ static int hw_break_set(struct task_stru
>>>>  			return ret;
>>>>  		offset += PTRACE_HBP_CTRL_SZ;
>>>>  
>>>> -		ret = user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
>>>> -						offset,
>>>> -						offset + PTRACE_HBP_PAD_SZ);
>>>> -		if (ret)
>>>> -			return ret;
>>>> +		user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
>>>> +					  offset, offset + PTRACE_HBP_PAD_SZ);
>>>>  		offset += PTRACE_HBP_PAD_SZ;
>>>>  		idx++;
>>>>  	}
>>>> @@ -939,10 +934,7 @@ static int sve_set_common(struct task_st
>>>>  
>>>>  	start = end;
>>>>  	end = SVE_PT_SVE_FPSR_OFFSET(vq);
>>>> -	ret = user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
>>>> -					start, end);
>>>> -	if (ret)
>>>> -		goto out;
>>>> +	user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf, start, end);
>>>
>>> I think it would be better to have user_regset_copyin_ignore() return void
>>> so that we don't run the risk of missing an error code if it starts
>>> returning one in future.
>>
>>    That's the plan! But I need to convert the users 1st, right?
> 
> Right, though normally we'd like to see the full series that does the
> arch clean-up followed by the user_regset_copyin_ignore() return changed
> to void. If for some reason the last patch is rejected by the maintainer
> because there are plans to actually return some non-zero value, we'd
> have to revert the above.

   Hm... I usually try to avoid a cross-tree patch series but here I should
be able to use the linux-next repo as I series' base. Is that OK?

MBR, Sergey