Message ID | 20250314-drm-bridge-refcount-v7-0-152571f8c694@bootlin.com (mailing list archive) |
---|---|
Headers | show |
Series | drm/bridge: add devm_drm_bridge_alloc() with bridge refcount | expand |
Hi, On Fri, Mar 14, 2025 at 11:31:13AM +0100, Luca Ceresoli wrote: > This series improves the way DRM bridges are allocated and initialized and > makes them reference-counted. The goal of reference counting is to avoid > use-after-free by drivers which got a pointer to a bridge and keep it > stored and used even after the bridge has been deallocated. > > The overall goal is supporting Linux devices with a DRM pipeline whose > final components can be hot-plugged and hot-unplugged, including one or > more bridges. For more details see the big picture [0]. > > DRM bridge drivers will have to be adapted to the new API, which is pretty > simple for most cases. Refcounting will have to be adopted on the two > sides: all functions returning a bridge pointer and all code obtaining such > a pointer. This series has just an overview of some of those conversions, > because for now the main goal is to agree on the API. > > Series layout: > > 1. Add the new API and refcounting: > > drm/bridge: add devm_drm_bridge_alloc() > drm/bridge: add support for refcounting > > 2. get/put the reference in basic operations in the bridge core: > > drm/bridge: get/put the bridge reference in drm_bridge_add/remove() > drm/bridge: get/put the bridge reference in drm_bridge_attach/detach() > > 3. as an example of changes for bridge consumers, get a reference for the > bridge returned by drm_bridge_chain_get_first_bridge(), have it put by > all callers (all users will be covered later on separately): > > drm/bridge: add a cleanup action for scope-based drm_bridge_put() invocation > drm/bridge: get the bridge returned by drm_bridge_chain_get_first_bridge() > drm/mxsfb: put the bridge returned by drm_bridge_chain_get_first_bridge() > drm/atomic-helper: put the bridge returned by drm_bridge_chain_get_first_bridge() > drm/probe-helper: put the bridge returned by drm_bridge_chain_get_first_bridge() > > 4. convert a few bridge drivers (bridge providers) to the new API: > > drm/bridge: ti-sn65dsi83: use dynamic lifetime management > drm/bridge: samsung-dsim: use dynamic lifetime management > > This work was formerly a part of my v6 DRM bridge hotplug series[0], now > split as a standalone series with many improvements, hence the "v7" version > number. Except for one patch where I had comments, I think the series is in excellent shape. We're still missing a couple of things to close this topic though: - Converting the other bridge iterators/accessors to take / put the references - Mass converting the drivers to devm_drm_bridge_alloc - Documenting somewhere (possibly in drm_bridge_init?) that it really shouldn't be used anymore Maxime
Hello Maxime, On Fri, 14 Mar 2025 19:21:01 +0100 Maxime Ripard <mripard@kernel.org> wrote: > Hi, > > On Fri, Mar 14, 2025 at 11:31:13AM +0100, Luca Ceresoli wrote: > > This series improves the way DRM bridges are allocated and > > initialized and makes them reference-counted. The goal of reference > > counting is to avoid use-after-free by drivers which got a pointer > > to a bridge and keep it stored and used even after the bridge has > > been deallocated. > > > > The overall goal is supporting Linux devices with a DRM pipeline > > whose final components can be hot-plugged and hot-unplugged, > > including one or more bridges. For more details see the big picture > > [0]. > > > > DRM bridge drivers will have to be adapted to the new API, which is > > pretty simple for most cases. Refcounting will have to be adopted > > on the two sides: all functions returning a bridge pointer and all > > code obtaining such a pointer. This series has just an overview of > > some of those conversions, because for now the main goal is to > > agree on the API. > > > > Series layout: > > > > 1. Add the new API and refcounting: > > > > drm/bridge: add devm_drm_bridge_alloc() > > drm/bridge: add support for refcounting > > > > 2. get/put the reference in basic operations in the bridge core: > > > > drm/bridge: get/put the bridge reference in > > drm_bridge_add/remove() drm/bridge: get/put the bridge reference in > > drm_bridge_attach/detach() > > > > 3. as an example of changes for bridge consumers, get a reference > > for the bridge returned by drm_bridge_chain_get_first_bridge(), > > have it put by all callers (all users will be covered later on > > separately): > > > > drm/bridge: add a cleanup action for scope-based > > drm_bridge_put() invocation drm/bridge: get the bridge returned by > > drm_bridge_chain_get_first_bridge() drm/mxsfb: put the bridge > > returned by drm_bridge_chain_get_first_bridge() drm/atomic-helper: > > put the bridge returned by drm_bridge_chain_get_first_bridge() > > drm/probe-helper: put the bridge returned by > > drm_bridge_chain_get_first_bridge() > > > > 4. convert a few bridge drivers (bridge providers) to the new API: > > > > drm/bridge: ti-sn65dsi83: use dynamic lifetime management > > drm/bridge: samsung-dsim: use dynamic lifetime management > > > > This work was formerly a part of my v6 DRM bridge hotplug > > series[0], now split as a standalone series with many improvements, > > hence the "v7" version number. > > Except for one patch where I had comments, I think the series is in > excellent shape. We're still missing a couple of things to close this > topic though: > > - Converting the other bridge iterators/accessors to take / put the > references I sent a couple in this series as you had asked, to show how conversion looks like. But I have a large part of this conversion partially done already, and it is the largest part of the refcounting work in terms of touched files due to the large number of drivers using the iterators and accessors. Here are the functions to convert: A) drm_bridge_chain_get_first_bridge B) drm_bridge_get_prev_bridge C) drm_bridge_get_next_bridge D) drm_for_each_bridge_in_chain E) drm_bridge_connector_init F) of_drm_find_bridge A) is present in this series as an example but I don't think it should be applied until all bridge drivers are converted to drm_bridge_alloc(). Otherwise for not-yet-converted bridge drivers we'd have drm_bridge_get/put() operating on an uninitialized kref, and __drm_bridge_free() called on non-refcounted bridges, so I think we'd see fireworks. In the previous iteration I used drm_bridge_is_refcounted() in drm_bridge_get/put() to allow a progressive migration, but if we want to convert everything in a single run we need to first convert all bridges to drm_bridge_alloc() and then convert all accessors. The same reasoning applies to patches 3-4 which add get/put to drm_bridge_add/remove() and _attach/detach(). B) to E) are ready in my work branch, about 20 patches in total. Indeed item E) is a special case but it is handled there too. Item F) is the beast, because of the reverse call graph of of_drm_find_bridge() which includes drm_of_find_panel_or_bridge() and then *_of_get_bridge(), each having a few dozen callers, and leading to the panel_bridge topic. I have converted maybe half of the users of accessors in F), it's 35 patches but it's the easy half and I still need to tackle to hardest ones. > - Mass converting the drivers to devm_drm_bridge_alloc Again I sent a couple in this series as you had asked, to show how conversion looks like for the typical bridge driver. There are ~70 drivers to convert in total and I think most will be easy as the two examples presented here. I think this should be merged entirely before merging any accessor changes, as explained above. > - Documenting somewhere (possibly in drm_bridge_init?) that it > really shouldn't be used anymore I'm afraid there is no drm_bridge_init(), bridge drivers just do [devm_]kzalloc and set fields explicitly. So I don't think there is a place to document this. However I used to have a documentation patch until v6 [0], and I think it should be revived and resent at some point, after removing the "legacy mode" as we are converting all drivers at once. BTW I also have a kunittest patch that should be revived. Do you still prefer me to resend these two patches as a separate series, waiting after the API in this series is applied? Overall, I think this could be the path forward, let me know if youthink it should be done differently: A. have patches 1 and 2 of this series applied (why not, even patches 10-11) B. after (A), send series to convert all bridge drivers to new API (includes patches 10-11 of this series if not applied already) C. after (A), send documentation and kunittest patches D. after (B), add get/put to drm_bridge_add/remove() + attach/detech() (patches 3-4 in this series) E. after (B), send series to convert accessors (including patches 5-9 in this series which convert drm_bridge_chain_get_first_bridge() and its users) [0] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-18-9d6f2c9c3058@bootlin.com/ Luca
On Mon, Mar 17, 2025 at 03:56:07PM +0100, Luca Ceresoli wrote: > Hello Maxime, > > On Fri, 14 Mar 2025 19:21:01 +0100 > Maxime Ripard <mripard@kernel.org> wrote: > > > Hi, > > > > On Fri, Mar 14, 2025 at 11:31:13AM +0100, Luca Ceresoli wrote: > > > This series improves the way DRM bridges are allocated and > > > initialized and makes them reference-counted. The goal of reference > > > counting is to avoid use-after-free by drivers which got a pointer > > > to a bridge and keep it stored and used even after the bridge has > > > been deallocated. > > > > > > The overall goal is supporting Linux devices with a DRM pipeline > > > whose final components can be hot-plugged and hot-unplugged, > > > including one or more bridges. For more details see the big picture > > > [0]. > > > > > > DRM bridge drivers will have to be adapted to the new API, which is > > > pretty simple for most cases. Refcounting will have to be adopted > > > on the two sides: all functions returning a bridge pointer and all > > > code obtaining such a pointer. This series has just an overview of > > > some of those conversions, because for now the main goal is to > > > agree on the API. > > > > > > Series layout: > > > > > > 1. Add the new API and refcounting: > > > > > > drm/bridge: add devm_drm_bridge_alloc() > > > drm/bridge: add support for refcounting > > > > > > 2. get/put the reference in basic operations in the bridge core: > > > > > > drm/bridge: get/put the bridge reference in > > > drm_bridge_add/remove() drm/bridge: get/put the bridge reference in > > > drm_bridge_attach/detach() > > > > > > 3. as an example of changes for bridge consumers, get a reference > > > for the bridge returned by drm_bridge_chain_get_first_bridge(), > > > have it put by all callers (all users will be covered later on > > > separately): > > > > > > drm/bridge: add a cleanup action for scope-based > > > drm_bridge_put() invocation drm/bridge: get the bridge returned by > > > drm_bridge_chain_get_first_bridge() drm/mxsfb: put the bridge > > > returned by drm_bridge_chain_get_first_bridge() drm/atomic-helper: > > > put the bridge returned by drm_bridge_chain_get_first_bridge() > > > drm/probe-helper: put the bridge returned by > > > drm_bridge_chain_get_first_bridge() > > > > > > 4. convert a few bridge drivers (bridge providers) to the new API: > > > > > > drm/bridge: ti-sn65dsi83: use dynamic lifetime management > > > drm/bridge: samsung-dsim: use dynamic lifetime management > > > > > > This work was formerly a part of my v6 DRM bridge hotplug > > > series[0], now split as a standalone series with many improvements, > > > hence the "v7" version number. > > > > Except for one patch where I had comments, I think the series is in > > excellent shape. We're still missing a couple of things to close this > > topic though: > > > > - Converting the other bridge iterators/accessors to take / put the > > references > > I sent a couple in this series as you had asked, to show how conversion > looks like. But I have a large part of this conversion partially done > already, and it is the largest part of the refcounting work in terms of > touched files due to the large number of drivers using the iterators > and accessors. Here are the functions to convert: > > A) drm_bridge_chain_get_first_bridge > B) drm_bridge_get_prev_bridge > C) drm_bridge_get_next_bridge > D) drm_for_each_bridge_in_chain > E) drm_bridge_connector_init > F) of_drm_find_bridge > > A) is present in this series as an example but I don't think it should > be applied until all bridge drivers are converted to > drm_bridge_alloc(). Otherwise for not-yet-converted bridge drivers we'd > have drm_bridge_get/put() operating on an uninitialized kref, and > __drm_bridge_free() called on non-refcounted bridges, so I think we'd > see fireworks. > > In the previous iteration I used drm_bridge_is_refcounted() in > drm_bridge_get/put() to allow a progressive migration, but if we want > to convert everything in a single run we need to first convert all > bridges to drm_bridge_alloc() and then convert all accessors. > > The same reasoning applies to patches 3-4 which add get/put to > drm_bridge_add/remove() and _attach/detach(). Agreed. > B) to E) are ready in my work branch, about 20 patches in total. > Indeed item E) is a special case but it is handled there too. > > Item F) is the beast, because of the reverse call graph of > of_drm_find_bridge() which includes drm_of_find_panel_or_bridge() and > then *_of_get_bridge(), each having a few dozen callers, and leading > to the panel_bridge topic. I have converted maybe half of the users of > accessors in F), it's 35 patches but it's the easy half and I still need > to tackle to hardest ones. One thing to keep in mind is that, while it's not correct, if the bridge has been allocated with devm_drm_bridge_alloc, it's not worse either. If you're not getting a reference to your pointer, the point is buggy, sure, but it's just as buggy as before, and in the same situations. So we can make that gradually if it's more convenient. One way to solve it would be that, for example, of_drm_find_bridge is oddly named according to our convention and it would make more sense to be called drm_of_find_bridge(). So maybe we can just create drm_of_find_bridge() that takes a reference, make of_drm_find_bridge() deprecated in favour of drm_of_find_bridge(), add a TODO, and call it a day. People will gradually switch to the new API over time. > > - Mass converting the drivers to devm_drm_bridge_alloc > > Again I sent a couple in this series as you had asked, to show how > conversion looks like for the typical bridge driver. There are ~70 > drivers to convert in total and I think most will be easy as the two > examples presented here. > > I think this should be merged entirely before merging any accessor > changes, as explained above. Agreed. > > - Documenting somewhere (possibly in drm_bridge_init?) that it > > really shouldn't be used anymore > > I'm afraid there is no drm_bridge_init(), bridge drivers just do > [devm_]kzalloc and set fields explicitly. So I don't think there is a > place to document this. Oh, right. Then, drm_bridge_add() would be a good candidate too to mention that bridges must be allocated using devm_drm_bridge_alloc(). > However I used to have a documentation patch until v6 [0], and I think > it should be revived and resent at some point, after removing the > "legacy mode" as we are converting all drivers at once. BTW I also have > a kunittest patch that should be revived. Do you still prefer me to > resend these two patches as a separate series, waiting after the API in > this series is applied? Both options work for me. > Overall, I think this could be the path forward, let me know if > youthink it should be done differently: > > A. have patches 1 and 2 of this series applied > (why not, even patches 10-11) I had some comments on patch 2, but it's ok for me on principle. > B. after (A), send series to convert all bridge drivers to new API > (includes patches 10-11 of this series if not applied already) > C. after (A), send documentation and kunittest patches > D. after (B), add get/put to drm_bridge_add/remove() + attach/detech() > (patches 3-4 in this series) > E. after (B), send series to convert accessors (including patches 5-9 > in this series which convert drm_bridge_chain_get_first_bridge() > and its users) Sounds like a plan. Maxime
Hello Mazime, On Wed, 19 Mar 2025 17:16:53 +0100 Maxime Ripard <mripard@kernel.org> wrote: > On Mon, Mar 17, 2025 at 03:56:07PM +0100, Luca Ceresoli wrote: > > Hello Maxime, > > > > On Fri, 14 Mar 2025 19:21:01 +0100 > > Maxime Ripard <mripard@kernel.org> wrote: > > > > > Hi, > > > > > > On Fri, Mar 14, 2025 at 11:31:13AM +0100, Luca Ceresoli wrote: > > > > This series improves the way DRM bridges are allocated and > > > > initialized and makes them reference-counted. The goal of reference > > > > counting is to avoid use-after-free by drivers which got a pointer > > > > to a bridge and keep it stored and used even after the bridge has > > > > been deallocated. > > > > > > > > The overall goal is supporting Linux devices with a DRM pipeline > > > > whose final components can be hot-plugged and hot-unplugged, > > > > including one or more bridges. For more details see the big picture > > > > [0]. > > > > > > > > DRM bridge drivers will have to be adapted to the new API, which is > > > > pretty simple for most cases. Refcounting will have to be adopted > > > > on the two sides: all functions returning a bridge pointer and all > > > > code obtaining such a pointer. This series has just an overview of > > > > some of those conversions, because for now the main goal is to > > > > agree on the API. > > > > > > > > Series layout: > > > > > > > > 1. Add the new API and refcounting: > > > > > > > > drm/bridge: add devm_drm_bridge_alloc() > > > > drm/bridge: add support for refcounting > > > > > > > > 2. get/put the reference in basic operations in the bridge core: > > > > > > > > drm/bridge: get/put the bridge reference in > > > > drm_bridge_add/remove() drm/bridge: get/put the bridge reference in > > > > drm_bridge_attach/detach() > > > > > > > > 3. as an example of changes for bridge consumers, get a reference > > > > for the bridge returned by drm_bridge_chain_get_first_bridge(), > > > > have it put by all callers (all users will be covered later on > > > > separately): > > > > > > > > drm/bridge: add a cleanup action for scope-based > > > > drm_bridge_put() invocation drm/bridge: get the bridge returned by > > > > drm_bridge_chain_get_first_bridge() drm/mxsfb: put the bridge > > > > returned by drm_bridge_chain_get_first_bridge() drm/atomic-helper: > > > > put the bridge returned by drm_bridge_chain_get_first_bridge() > > > > drm/probe-helper: put the bridge returned by > > > > drm_bridge_chain_get_first_bridge() > > > > > > > > 4. convert a few bridge drivers (bridge providers) to the new API: > > > > > > > > drm/bridge: ti-sn65dsi83: use dynamic lifetime management > > > > drm/bridge: samsung-dsim: use dynamic lifetime management > > > > > > > > This work was formerly a part of my v6 DRM bridge hotplug > > > > series[0], now split as a standalone series with many improvements, > > > > hence the "v7" version number. > > > > > > Except for one patch where I had comments, I think the series is in > > > excellent shape. We're still missing a couple of things to close this > > > topic though: > > > > > > - Converting the other bridge iterators/accessors to take / put the > > > references > > > > I sent a couple in this series as you had asked, to show how conversion > > looks like. But I have a large part of this conversion partially done > > already, and it is the largest part of the refcounting work in terms of > > touched files due to the large number of drivers using the iterators > > and accessors. Here are the functions to convert: > > > > A) drm_bridge_chain_get_first_bridge > > B) drm_bridge_get_prev_bridge > > C) drm_bridge_get_next_bridge > > D) drm_for_each_bridge_in_chain > > E) drm_bridge_connector_init > > F) of_drm_find_bridge > > > > A) is present in this series as an example but I don't think it should > > be applied until all bridge drivers are converted to > > drm_bridge_alloc(). Otherwise for not-yet-converted bridge drivers we'd > > have drm_bridge_get/put() operating on an uninitialized kref, and > > __drm_bridge_free() called on non-refcounted bridges, so I think we'd > > see fireworks. > > > > In the previous iteration I used drm_bridge_is_refcounted() in > > drm_bridge_get/put() to allow a progressive migration, but if we want > > to convert everything in a single run we need to first convert all > > bridges to drm_bridge_alloc() and then convert all accessors. > > > > The same reasoning applies to patches 3-4 which add get/put to > > drm_bridge_add/remove() and _attach/detach(). > > Agreed. > > > B) to E) are ready in my work branch, about 20 patches in total. > > Indeed item E) is a special case but it is handled there too. > > > > Item F) is the beast, because of the reverse call graph of > > of_drm_find_bridge() which includes drm_of_find_panel_or_bridge() and > > then *_of_get_bridge(), each having a few dozen callers, and leading > > to the panel_bridge topic. I have converted maybe half of the users of > > accessors in F), it's 35 patches but it's the easy half and I still need > > to tackle to hardest ones. > > One thing to keep in mind is that, while it's not correct, if the bridge > has been allocated with devm_drm_bridge_alloc, it's not worse either. If > you're not getting a reference to your pointer, the point is buggy, > sure, but it's just as buggy as before, and in the same situations. > > So we can make that gradually if it's more convenient. I see your point, right. And definitely doing it in gradually will help. > One way to solve it would be that, for example, of_drm_find_bridge is > oddly named according to our convention and it would make more sense to > be called drm_of_find_bridge(). > > So maybe we can just create drm_of_find_bridge() that takes a reference, > make of_drm_find_bridge() deprecated in favour of drm_of_find_bridge(), > add a TODO, and call it a day. People will gradually switch to the new > API over time. Thanks for the suggestion, I will certainly try this way when I get back to the accessor conversion work. While increasing the number of deprecated functions is not great, I'm starting to suspect a conversion of _all_ the of_drm_find_bridge() users at once is not realistic. > > > - Documenting somewhere (possibly in drm_bridge_init?) that it > > > really shouldn't be used anymore > > > > I'm afraid there is no drm_bridge_init(), bridge drivers just do > > [devm_]kzalloc and set fields explicitly. So I don't think there is a > > place to document this. > > Oh, right. > > Then, drm_bridge_add() would be a good candidate too to mention that > bridges must be allocated using devm_drm_bridge_alloc(). Sure. I'll add such a comment. > > Overall, I think this could be the path forward, let me know if > > youthink it should be done differently: > > > > A. have patches 1 and 2 of this series applied > > (why not, even patches 10-11) > > I had some comments on patch 2, but it's ok for me on principle. Sorry, my wording was not clear. I really meant "have patches 1 and 2 of this series applied after the improvements proposed". So hopefully the v8 I'm sending today-ish. I'm going to send only patches 1, 2, 10 and 11. The other patches are not meant to be merged now, so I'm resending them when appropriate. Luca
This series improves the way DRM bridges are allocated and initialized and makes them reference-counted. The goal of reference counting is to avoid use-after-free by drivers which got a pointer to a bridge and keep it stored and used even after the bridge has been deallocated. The overall goal is supporting Linux devices with a DRM pipeline whose final components can be hot-plugged and hot-unplugged, including one or more bridges. For more details see the big picture [0]. DRM bridge drivers will have to be adapted to the new API, which is pretty simple for most cases. Refcounting will have to be adopted on the two sides: all functions returning a bridge pointer and all code obtaining such a pointer. This series has just an overview of some of those conversions, because for now the main goal is to agree on the API. Series layout: 1. Add the new API and refcounting: drm/bridge: add devm_drm_bridge_alloc() drm/bridge: add support for refcounting 2. get/put the reference in basic operations in the bridge core: drm/bridge: get/put the bridge reference in drm_bridge_add/remove() drm/bridge: get/put the bridge reference in drm_bridge_attach/detach() 3. as an example of changes for bridge consumers, get a reference for the bridge returned by drm_bridge_chain_get_first_bridge(), have it put by all callers (all users will be covered later on separately): drm/bridge: add a cleanup action for scope-based drm_bridge_put() invocation drm/bridge: get the bridge returned by drm_bridge_chain_get_first_bridge() drm/mxsfb: put the bridge returned by drm_bridge_chain_get_first_bridge() drm/atomic-helper: put the bridge returned by drm_bridge_chain_get_first_bridge() drm/probe-helper: put the bridge returned by drm_bridge_chain_get_first_bridge() 4. convert a few bridge drivers (bridge providers) to the new API: drm/bridge: ti-sn65dsi83: use dynamic lifetime management drm/bridge: samsung-dsim: use dynamic lifetime management This work was formerly a part of my v6 DRM bridge hotplug series[0], now split as a standalone series with many improvements, hence the "v7" version number. [0] https://lore.kernel.org/dri-devel/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/ Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> --- Luca Ceresoli (11): drm/bridge: add devm_drm_bridge_alloc() drm/bridge: add support for refcounting drm/bridge: get/put the bridge reference in drm_bridge_add/remove() drm/bridge: get/put the bridge reference in drm_bridge_attach/detach() drm/bridge: add a cleanup action for scope-based drm_bridge_put() invocation drm/bridge: get the bridge returned by drm_bridge_chain_get_first_bridge() drm/mxsfb: put the bridge returned by drm_bridge_chain_get_first_bridge() drm/atomic-helper: put the bridge returned by drm_bridge_chain_get_first_bridge() drm/probe-helper: put the bridge returned by drm_bridge_chain_get_first_bridge() drm/bridge: ti-sn65dsi83: use dynamic lifetime management drm/bridge: samsung-dsim: use dynamic lifetime management drivers/gpu/drm/bridge/samsung-dsim.c | 7 +- drivers/gpu/drm/bridge/ti-sn65dsi83.c | 7 +- drivers/gpu/drm/drm_atomic_helper.c | 5 ++ drivers/gpu/drm/drm_bridge.c | 74 +++++++++++++++++++-- drivers/gpu/drm/drm_probe_helper.c | 1 + drivers/gpu/drm/mxsfb/lcdif_kms.c | 3 +- include/drm/drm_bridge.h | 119 +++++++++++++++++++++++++++++++++- 7 files changed, 201 insertions(+), 15 deletions(-) --- base-commit: 9e6d91c60b0d64a4f945663993b3bbf4f3fb7392 change-id: 20250314-drm-bridge-refcount-58d9503503f6 Best regards,