On Fri, Feb 08, 2019 at 09:53:27PM +0200, Oded Gabbay wrote: > On Fri, Feb 8, 2019 at 2:06 PM Greg KH <gre...@linuxfoundation.org> wrote: > > > > On Mon, Feb 04, 2019 at 10:32:44PM +0200, Oded Gabbay wrote: > > > +int hl_cb_ioctl(struct hl_fpriv *hpriv, void *data) > > > +{ > > > + union hl_cb_args *args = data; > > > + struct hl_device *hdev = hpriv->hdev; > > > + u64 handle; > > > + int rc; > > > + > > > + switch (args->in.op) { > > > + case HL_CB_OP_CREATE: > > > + rc = hl_cb_create(hdev, &hpriv->cb_mgr, args->in.cb_size, > > > + &handle, hpriv->ctx->asid); > > > > so cb_size comes from userspace, ok, you check for the value to be too > > small, but not too big. That means someone can try to allocate too much > > memory, possibly crashing things, not good :( > Yes, correct, but even if I limit a single allocation to, let's say, > 1MB, what's stopping a userspace process from allocating multiple CBs > and draining the system memory ? I'm counting on the oom module to > kill that process if it mis-behaves. > And, btw, I assumed there is hard limit of 4MB on a single > dma_alloc_coherent. i.e. I was never able to allocate more then 4MB > through that API. So I never thought I need to check for max size > because of that hard limit. > Am I missing something here ?
Relying on the oom module to handle driver issues is not always a wise idea ;) You should put a bounds on your max allocation, if it really is 4MB, then test for that. thanks, greg k-h