netdev - Re: [PATCH rdma-next v2 01/13] IB/uverbs: Add an ib_uobject getter to ioctl() infrastructure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 May 2018 14:21:19 -0600
From:   Jason Gunthorpe <jgg@...lanox.com>
To:     "Ruhl, Michael J" <michael.j.ruhl@...el.com>
Cc:     Leon Romanovsky <leon@...nel.org>,
        Doug Ledford <dledford@...hat.com>,
        Leon Romanovsky <leonro@...lanox.com>,
        RDMA mailing list <linux-rdma@...r.kernel.org>,
        Boris Pismenny <borisp@...lanox.com>,
        Matan Barak <matanb@...lanox.com>,
        Raed Salem <raeds@...lanox.com>,
        Yishai Hadas <yishaih@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        linux-netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH rdma-next v2 01/13] IB/uverbs: Add an ib_uobject getter
 to ioctl() infrastructure

On Tue, May 29, 2018 at 07:31:22PM +0000, Ruhl, Michael J wrote:
> >-	struct ib_uverbs_destroy_cq_resp resp;
> > 	struct ib_uobject *uobj =
> >-		uverbs_attr_get(attrs,
> >UVERBS_ATTR_DESTROY_CQ_HANDLE)->obj_attr.uobject;
> >-	struct ib_ucq_object *obj = container_of(uobj, struct ib_ucq_object,
> >-						 uobject);
> >+		uverbs_attr_get_uobject(attrs,
> >UVERBS_ATTR_DESTROY_CQ_HANDLE);
> >+	struct ib_uverbs_destroy_cq_resp resp;
> >+	struct ib_ucq_object *obj;
> > 	int ret;
> >
> >+	if (IS_ERR(uobj))
> >+		return PTR_ERR(uobj);
> >+
> 
> I remember a conversation that if an method attribute was mandatory, that you did not need to
> test the uobj for error (since it was checked in the infrastructure).

Yes.

> Is this error check necessary?

No

But there is no way to check one way or the other at compile time
right now, and omitting the check makes smatch mad.

We need some more patches to be able to safely omit the check...

Jason