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
| ||
|
Message-ID: <a869ec2e-966b-f8ea-7f68-a77c214a45af@quicinc.com> Date: Mon, 9 Oct 2023 17:29:40 +0530 From: Mukesh Ojha <quic_mojha@...cinc.com> To: Kees Cook <keescook@...omium.org> CC: <corbet@....net>, <agross@...nel.org>, <andersson@...nel.org>, <konrad.dybcio@...aro.org>, <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>, <tony.luck@...el.com>, <gpiccoli@...lia.com>, <mathieu.poirier@...aro.org>, <catalin.marinas@....com>, <will@...nel.org>, <linus.walleij@...aro.org>, <andy.shevchenko@...il.com>, <vigneshr@...com>, <nm@...com>, <matthias.bgg@...il.com>, <kgene@...nel.org>, <alim.akhtar@...sung.com>, <bmasney@...hat.com>, <quic_tsoni@...cinc.com>, <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>, <linux-hardening@...r.kernel.org>, <linux-remoteproc@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>, <linux-gpio@...r.kernel.org>, <linux-mediatek@...ts.infradead.org>, <linux-samsung-soc@...r.kernel.org>, <kernel@...cinc.com> Subject: Re: [REBASE PATCH v5 10/17] pstore: Add pstore_region_defined() helper and export it Thanks for the review. On 9/14/2023 4:54 AM, Kees Cook wrote: > On Mon, Sep 11, 2023 at 04:23:52PM +0530, Mukesh Ojha wrote: >> There are users like Qualcomm minidump which is interested in >> knowing the pstore frontend addresses and sizes from the backend >> (ram) to be able to register it with firmware to finally collect >> them during crash for debugging. >> >> Signed-off-by: Mukesh Ojha <quic_mojha@...cinc.com> >> --- >> fs/pstore/platform.c | 15 +++++++++++++++ >> fs/pstore/ram.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >> include/linux/pstore.h | 6 ++++++ >> 3 files changed, 63 insertions(+) >> >> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c >> index e5bca9a004cc..fdac951059c1 100644 >> --- a/fs/pstore/platform.c >> +++ b/fs/pstore/platform.c >> @@ -139,6 +139,21 @@ enum pstore_type_id pstore_name_to_type(const char *name) >> } >> EXPORT_SYMBOL_GPL(pstore_name_to_type); >> >> +int pstore_region_defined(struct pstore_record *record, >> + void **virt, phys_addr_t *phys, >> + size_t *size, unsigned int *max_dump_cnt) >> +{ >> + if (!psinfo) >> + return -EINVAL; >> + >> + record->psi = psinfo; > > Uh, this makes no sense to me. If this is a real pstore_record, you > cannot just assign psi here. Ok. > >> + >> + return psinfo->region_info ? >> + psinfo->region_info(record, virt, phys, size, max_dump_cnt) : >> + -EINVAL; > > Common code style for this kind of thing is usually like this: > > if (!psinfo->region_info) > return -EINVAL; > > return psinfo->region_info(...) Thanks. -Mukesh > >> +} >> +EXPORT_SYMBOL_GPL(pstore_region_defined); >> + >> static void pstore_timer_kick(void) >> { >> if (pstore_update_ms < 0) >> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c >> index ab551caa1d2a..62202f3ddf63 100644 >> --- a/fs/pstore/ram.c >> +++ b/fs/pstore/ram.c >> @@ -437,6 +437,47 @@ static int ramoops_pstore_erase(struct pstore_record *record) >> return 0; >> } >> >> +static int ramoops_region_info(struct pstore_record *record, >> + void **virt, phys_addr_t *phys, >> + size_t *size, unsigned int *max_dump_cnt) > > But there's a larger problem here -- "virt", "phys" and likely > "max_dump_cnt" are aspects _specific to the ram backend_. This can't be > a generic pstore interface. > > I'm not opposed to it being exposed only from ramoops, though. > > But I think you'll want a pstore generic way to iterate over the > records..
Powered by blists - more mailing lists