[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230602225519.66c2c987@kernel.org>
Date: Fri, 2 Jun 2023 22:55:19 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Ding Hui <dinghui@...gfor.com.cn>
Cc: Alexander Duyck <alexander.duyck@...il.com>, Andrew Lunn
<andrew@...n.ch>, davem@...emloft.net, edumazet@...gle.com,
pabeni@...hat.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
pengdonglin@...gfor.com.cn, huangcun@...gfor.com.cn
Subject: Re: [PATCH net-next] net: ethtool: Fix out-of-bounds copy to user
On Sat, 3 Jun 2023 09:51:34 +0800 Ding Hui wrote:
> > If that is the case maybe it would just make more sense to just return
> > an error if we are at risk of overrunning the userspace allocated
> > buffer.
>
> In that case, I can modify to return an error, however, I think the
> ENOSPC or EFBIG mentioned in a previous email may not be suitable,
> maybe like others length/size checking return EINVAL.
>
> Another thing I wondered is that should I update the current length
> back to user if user buffer is not enough, assuming we update the new
> length with error returned, the userspace can use it to reallocate
> buffer if he wants to, which can avoid re-call previous ioctl to get
> the new length.
This entire thread presupposes that user provides the length of
the buffer. I don't see that in the code. Take ethtool_get_stats()
as an example, you assume that stats.n_stats is set correctly,
but it's not enforced today. Some app somewhere may pass in zeroed
out stats and work just fine.
Powered by blists - more mailing lists