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: <29f9a2ff1979406489213909b940184f@realtek.com> Date: Mon, 16 Oct 2023 09:15:51 +0000 From: Hayes Wang <hayeswang@...ltek.com> To: Douglas Anderson <dianders@...omium.org>, Jakub Kicinski <kuba@...nel.org>, "David S . Miller" <davem@...emloft.net> CC: Alan Stern <stern@...land.harvard.edu>, Simon Horman <horms@...nel.org>, Edward Hill <ecgh@...omium.org>, Laura Nao <laura.nao@...labora.com>, "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, Grant Grundler <grundler@...omium.org>, Bjørn Mork <bjorn@...k.no>, "Eric Dumazet" <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: RE: [PATCH v3 5/5] r8152: Block future register access if register access fails Douglas Anderson <dianders@...omium.org> > Sent: Friday, October 13, 2023 3:25 AM [...] > static int generic_ocp_read(struct r8152 *tp, u16 index, u16 size, > @@ -8265,6 +8353,19 @@ static int rtl8152_pre_reset(struct usb_interface > *intf) > if (!tp) > return 0; > > + /* We can only use the optimized reset if we made it to the end of > + * probe without any register access fails, which sets > + * `PROBED_WITH_NO_ERRORS` to true. If we didn't have that then return > + * an error here which tells the USB framework to fully unbind/rebind > + * our driver. Would you stay in a loop of unbind and rebind, if the control transfers in the probe() are not always successful? I just think about the worst case that at least one control always fails in probe(). > + */ > + mutex_lock(&tp->control); I don't think you need the mutex for testing the bit. > + if (!test_bit(PROBED_WITH_NO_ERRORS, &tp->flags)) { > + mutex_unlock(&tp->control); > + return -EIO; > + } > + mutex_unlock(&tp->control); > + > netdev = tp->netdev; > if (!netif_running(netdev)) > return 0; > @@ -8277,7 +8378,9 @@ static int rtl8152_pre_reset(struct usb_interface > *intf) > napi_disable(&tp->napi); > if (netif_carrier_ok(netdev)) { > mutex_lock(&tp->control); > + set_bit(IN_PRE_RESET, &tp->flags); > tp->rtl_ops.disable(tp); > + clear_bit(IN_PRE_RESET, &tp->flags); > mutex_unlock(&tp->control); > } > > @@ -8293,6 +8396,10 @@ static int rtl8152_post_reset(struct usb_interface > *intf) > if (!tp) > return 0; > > + mutex_lock(&tp->control); I don't think clear_bit() needs the protection of mutex. I think you could call rtl_set_accessible() directly. > + rtl_set_accessible(tp); > + mutex_unlock(&tp->control); > + > /* reset the MAC address in case of policy change */ > if (determine_ethernet_addr(tp, &sa) >= 0) { > rtnl_lock(); Best Regards, Hayes
Powered by blists - more mailing lists