Quoting Tvrtko Ursulin (2019-02-25 10:41:28)
>
> On 06/02/2019 13:03, Chris Wilson wrote:
> > +static int
> > +get_engines(struct i915_gem_context *ctx,
> > + struct drm_i915_gem_context_param *args)
> > +{
> > + struct i915_context_param_engines *local;
> > + unsigned int n, count, size;
> > + int err;
> > +
> > +restart:
> > + count = READ_ONCE(ctx->nengine);
> > + if (count > (INT_MAX - sizeof(*local)) / sizeof(*local->class_instance))
> > + return -ENOMEM; /* unrepresentable! */
> > +
> > + size = sizeof(*local) + count * sizeof(*local->class_instance);
> > + if (!args->size) {
> > + args->size = size;
> > + return 0;
> > + }
> > + if (args->size < size)
> > + return -EINVAL;
> > +
> > + local = kmalloc(size, GFP_KERNEL);
> > + if (!local)
> > + return -ENOMEM;
> > +
> > + if (mutex_lock_interruptible(&ctx->i915->drm.struct_mutex)) {
> > + err = -EINTR;
> > + goto err;
> > + }
> > +
> > + if (READ_ONCE(ctx->nengine) != count) {
> > + mutex_unlock(&ctx->i915->drm.struct_mutex);
> > + kfree(local);
> > + goto restart;
> > + }
> > +
> > + local->extensions = 0;
> > + for (n = 0; n < count; n++) {
> > + if (ctx->engines[n]) {
> > + local->class_instance[n].engine_class =
> > + ctx->engines[n]->uabi_class;
> > + local->class_instance[n].engine_instance =
> > + ctx->engines[n]->instance;
> > + } else {
> > + local->class_instance[n].engine_class =
> > + I915_ENGINE_CLASS_INVALID;
> > + local->class_instance[n].engine_instance =
> > + I915_ENGINE_CLASS_INVALID_NONE;
> > + }
> > + }
> > +
> > + mutex_unlock(&ctx->i915->drm.struct_mutex);
> > +
> > + if (copy_to_user(u64_to_user_ptr(args->value), local, size)) {
> > + err = -EFAULT;
> > + goto err;
> > + }
> > +
>
> Sripada, Radhakrishna <radhakrishna.sripada@xxxxxxxxx> reports leakage
> of local on the success path here.
>
> Alternatively, perhaps you could simplify by using stack space, with
> some reasonable max size for future proofing, and a GEM_WARN_ON error
> return or something.
I don't expect this getter to be called frequently, so resisting the urge
to optimise. And similarly not impose restrictions we may need to lift
later on. For once, kiss.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
