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
| ||
|
Date: Mon, 28 Mar 2016 17:05:24 +0900 From: Krzysztof Kozlowski <k.kozlowski@...sung.com> To: Andy Yan <andy.yan@...k-chips.com> Cc: robh+dt@...nel.org, sre@...nel.org, heiko@...ech.de, john.stultz@...aro.org, arnd@...db.de, galak@...eaurora.org, ijc+devicetree@...lion.org.uk, catalin.marinas@....com, olof@...om.net, alexandre.belloni@...e-electrons.com, dbaryshkov@...il.com, jun.nie@...aro.org, pawel.moll@....com, will.deacon@....com, linux-rockchip@...ts.infradead.org, matthias.bgg@...il.com, devicetree@...r.kernel.org, linux-pm@...r.kernel.org, f.fainelli@...il.com, linux@....linux.org.uk, mbrugger@...e.com, linux-arm-kernel@...ts.infradead.org, lorenzo.pieralisi@....com, moritz.fischer@...us.com, linux-kernel@...r.kernel.org, wxt@...k-chips.com, dwmw2@...radead.org, mark.rutland@....com Subject: Re: [PATCH v6 2/4] power: reset: add reboot mode driver On 28.03.2016 16:40, Andy Yan wrote: > Hi Krzysztof : > > On 2016年03月28日 14:34, Krzysztof Kozlowski wrote: >> On 24.03.2016 17:03, Andy Yan wrote: >>> Hi Krzystof: >> (...) >> >>>>> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, >>>>> + const char *cmd) >>>>> +{ >>>>> + const char *normal = "normal"; >>>>> + int magic = 0; >>>>> + struct mode_info *info; >>>>> + >>>>> + if (!cmd) >>>>> + cmd = normal; >>>>> + >>>>> + list_for_each_entry(info, &reboot->head, list) { >>>>> + if (!strcmp(info->mode, cmd)) { >>>>> + magic = info->magic; >>>>> + break; >>>>> + } >>>>> + } >>>>> + >>>>> + return magic; >>>> In absence of 'normal' mode (it is not described as required property) >>>> the magic will be '0'. It would be nice to document that in bindings. >>>> Imagine someone forgets about this and will wonder why 0x0 is written >>>> to his precious register on normal reboot... >>> If the magic value is '0', we won't touch the register, please see >>> reboot_mode_notify bellow. >> Ah, indeed... so we cannot use value of '0' for magic (e.g. to clear any >> existing value for normal reboot)? > > It seems that the value '0' cannot be used. How about mentioning it in bindings documentation? (...) >>>>> + strcpy(info->mode, prop->name + len); >>>> Ehm, and how do you protect that name of mode is shorter than 32 >>>> characters? >>> How about info->mode = prop->name + len ? >> I don't get your answer. >> As fair as I read the code, the prop->name can be very long and you are >> copying it from 5 character. If the name of the mode has bazillion >> characters then again my question: how do you protect that it will fit >> in 32 bytes of 'mode'? > > What I mean is set info->mode as a pointer point to prop->name + len > > struct mode_info { > char *mode; > .......... > ......... > } > > info->mode = prop->name + len Ahh, I get it. Then I guess you should also do of_node_get() and of_node_put()... and use kstrdup_const(). Looks good but I am not familiar with overlays and life-cycle of OF nodes (documentation for the life-cycle is a todo list item: Documentation/devicetree/todo.txt). Best regards, Krzysztof
Powered by blists - more mailing lists