Re: [PATCH 2/2] ARM: qcom_defconfig: Enable Qualcomm remoteproc and SMP2P drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 07, 2017 at 12:04:13PM +0100, Bjorn Andersson wrote:
> On Tue 07 Mar 03:01 CET 2017, Jonathan Neusch?fer wrote:
[...]
> If this is something that you will continue to hack on and you think
> anyone else will be interested in having then please do submit patches
> for it.

Ok, I'll give it a try.

> > > > [    0.647743] qcom-smsm smsm: no smsm size info, using defaults
> > > > [    0.647775] qcom-smsm smsm: unable to allocate shared state entry
> > > > 
> > > 
> > > Could you please confirm where in qcom_smem_alloc_global() we're
> > > failing? As far as I can tell we should fail with -EEXIST or if the
> > > passed "size" parameter is bogus -ENOMEM (but the default number of
> > > entries really should be less than the amount of free SMEM space).
> > 
> > * qcom_smem_get returns -EPROBE_DEFER:
> > 
> > 	void *ptr = ERR_PTR(-EPROBE_DEFER);
> > 	if (!__smem)
> > 		return ptr;
> > 
> > * smsm_get_size_info prints "no smsm size info, using defaults\n"
> > * qcom_smem_alloc also returns -EPROBE_DEFER early.
> > 
> > 
> > BTW, I think smsm_get_size_info is using uninitialized memory here:
> > 
> >         size_t size;    /* is uninitialized */
> >         struct { ... } *info;
> > 
> >         /* qcom_smem_get returns early without setting size */
> >         info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_SMSM_SIZE_INFO, &size);
> > 
> >         /*
> > 	 * PTR_ERR(info) is not -ENOENT.
> >          * size (still uninitialized) is compared with the size of the local
> >          * struct defined above.
> > 	 */
> >         if (PTR_ERR(info) == -ENOENT || size != sizeof(*info)) {
> >                 ...
> >         }
> > 
> 
> Thanks for your analysis!
> 
> As you say the smsm driver is missing handling of probe deferral - which
> is wrong. Could you please propose a patch checking for EPROBE_DEFER and
> propagate this error as return value from probe()?  (Without an error
> message)

I'm working on it.

Regarding the uninitialized memory read in "size != sizeof(*info)":
Is qcom_smem_get supposed to store a valid size value in any
non-probe-deferral case, or only in non-error cases?

Assuming the latter, I think "size != sizeof(*info)" should be changed
to "!IS_ERR(info) && size != sizeof(*info)", i.e. only check size if
qcom_smem_get returned success. Does that seem reasonable?



Jonathan Neuschäfer

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux