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: Thu, 24 Nov 2022 10:12:15 +0100 From: Jiri Pirko <jiri@...nulli.us> To: Jacob Keller <jacob.e.keller@...el.com> Cc: netdev@...r.kernel.org, Jiri Pirko <jiri@...dia.com>, Jakub Kicinski <kuba@...nel.org> Subject: Re: [PATCH net-next v2 5/9] devlink: refactor region_read_snapshot_fill to use a callback function Wed, Nov 23, 2022 at 09:38:30PM CET, jacob.e.keller@...el.com wrote: >The devlink_nl_region_read_snapshot_fill is used to copy the contents of >a snapshot into a message for reporting to userspace via the >DEVLINK_CMG_REGION_READ netlink message. > >A future change is going to add support for directly reading from >a region. Almost all of the logic for this new capability is identical. > >To help reduce code duplication and make this logic more generic, >refactor the function to take a cb and cb_priv pointer for doing the >actual copy. > >Add a devlink_region_snapshot_fill implementation that will simply copy >the relevant chunk of the region. This does require allocating some >storage for the chunk as opposed to simply passing the correct address >forward to the devlink_nl_cmg_region_read_chunk_fill function. > >A future change to implement support for directly reading from a region >without a snapshot will provide a separate implementation that calls the >newly added devlink region operation. > >Signed-off-by: Jacob Keller <jacob.e.keller@...el.com> >--- >Changes since v1: >* Use kmalloc instead of kzalloc >* Don't combine data_size declaration and assignment >* Fix the always_unused placement for devlink_region_snapshot_fill > > net/core/devlink.c | 44 +++++++++++++++++++++++++++++++++++--------- > 1 file changed, 35 insertions(+), 9 deletions(-) > >diff --git a/net/core/devlink.c b/net/core/devlink.c >index bd7af0600405..729e2162a4db 100644 >--- a/net/core/devlink.c >+++ b/net/core/devlink.c >@@ -6460,25 +6460,36 @@ static int devlink_nl_cmd_region_read_chunk_fill(struct sk_buff *msg, > > #define DEVLINK_REGION_READ_CHUNK_SIZE 256 > >-static int devlink_nl_region_read_snapshot_fill(struct sk_buff *skb, >- struct devlink_snapshot *snapshot, >- u64 start_offset, >- u64 end_offset, >- u64 *new_offset) >+typedef int devlink_chunk_fill_t(void *cb_priv, u8 *chunk, u32 chunk_size, >+ u64 curr_offset, >+ struct netlink_ext_ack *extack); >+ >+static int >+devlink_nl_region_read_fill(struct sk_buff *skb, devlink_chunk_fill_t *cb, >+ void *cb_priv, u64 start_offset, u64 end_offset, >+ u64 *new_offset, struct netlink_ext_ack *extack) > { > u64 curr_offset = start_offset; > int err = 0; >+ u8 *data; >+ >+ /* Allocate and re-use a single buffer */ >+ data = kmalloc(DEVLINK_REGION_READ_CHUNK_SIZE, GFP_KERNEL); Hmm, I tried to figure out how to do this without extra alloc and memcpy, didn't find any nice solution :/ Reviewed-by: Jiri Pirko <jiri@...dia.com> Btw, do you plan to extend this for write as well. It might be valuable for debugging purposes to have that. I recall we discussed it in past. >+ if (!data) >+ return -ENOMEM; > > *new_offset = start_offset; > > while (curr_offset < end_offset) { > u32 data_size; >- u8 *data; > > data_size = min_t(u32, end_offset - curr_offset, > DEVLINK_REGION_READ_CHUNK_SIZE); > >- data = &snapshot->data[curr_offset]; >+ err = cb(cb_priv, data, data_size, curr_offset, extack); >+ if (err) >+ break; >+ > err = devlink_nl_cmd_region_read_chunk_fill(skb, data, data_size, curr_offset); > if (err) > break; >@@ -6487,9 +6498,23 @@ static int devlink_nl_region_read_snapshot_fill(struct sk_buff *skb, > } > *new_offset = curr_offset; > >+ kfree(data); >+ > return err; > } > >+static int >+devlink_region_snapshot_fill(void *cb_priv, u8 *chunk, u32 chunk_size, >+ u64 curr_offset, >+ struct netlink_ext_ack __always_unused *extack) >+{ >+ struct devlink_snapshot *snapshot = cb_priv; >+ >+ memcpy(chunk, &snapshot->data[curr_offset], chunk_size); >+ >+ return 0; >+} >+ > static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb, > struct netlink_callback *cb) > { >@@ -6608,8 +6633,9 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb, > goto nla_put_failure; > } > >- err = devlink_nl_region_read_snapshot_fill(skb, snapshot, start_offset, >- end_offset, &ret_offset); >+ err = devlink_nl_region_read_fill(skb, &devlink_region_snapshot_fill, >+ snapshot, start_offset, end_offset, >+ &ret_offset, cb->extack); > > if (err && err != -EMSGSIZE) > goto nla_put_failure; >-- >2.38.1.420.g319605f8f00e >
Powered by blists - more mailing lists