Loic Poulain <loic.poulain(a)linaro.org> writes:
> Hi Bryan,
>
> On Tue, 31 Aug 2021 at 03:13, Bryan O'Donoghue <bryan.odonoghue(a)linaro.org> wrote:
>
> On 30/08/2021 18:09, Loic Poulain wrote:
> > This reverts commit 8def9ec46a5fafc0abcf34489a9e8a787bca984d.
> >
> > The firmware keep-alive does not cause any event in case of error
> > such as non acked. It's just a basic keep alive to prevent the AP
> > to kick-off the station due to inactivity. So let mac80211 submit
> > its own monitoring packet (probe/null) and disconnect on timeout.
> >
> > Note: We want to keep firmware keep alive to prevent kick-off
> > when host is in suspend-to-mem (no mac80211 monitor packet).
> > Ideally fw keep alive should be enabled in suspend path and disabled
> > in resume path to prevent having both firmware and mac80211 submitting
> > periodic null packets.
> >
> > This fixes non detected AP leaving issues in active mode (nothing
> > monitors beacon or connection).
> >
> > Cc: stable(a)vger.kernel.org
> > Fixes: 8def9ec46a5f ("wcn36xx: Enable firmware link monitoring")
> > Signed-off-by: Loic Poulain <loic.poulain(a)linaro.org>
> > ---
> > drivers/net/wireless/ath/wcn36xx/main.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/wcn36xx/main.c
> b/drivers/net/wireless/ath/wcn36xx/main.c
> > index 216bc34..128d25d 100644
> > --- a/drivers/net/wireless/ath/wcn36xx/main.c
> > +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> > @@ -1362,7 +1362,6 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
> > ieee80211_hw_set(wcn->hw, HAS_RATE_CONTROL);
> > ieee80211_hw_set(wcn->hw, SINGLE_SCAN_ON_ALL_BANDS);
> > ieee80211_hw_set(wcn->hw, REPORTS_TX_ACK_STATUS);
> > - ieee80211_hw_set(wcn->hw, CONNECTION_MONITOR);
> >
> > wcn->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
> > BIT(NL80211_IFTYPE_AP) |
> >
>
> But why is BSS heartbeat offload not running, it should be.
>
> I agree we should switch this bit off for now since its obviously not
> working as intended.
>
> But we need to root cause _why_
>
> I think it has just not be designed as a connection tracking mechanism but as a simple
> keep alive, which is submitted every 30s unconditionally.
>
> In suspend absent a working heartbeat monitor - if the AP goes away we
> stay in suspend indefinitely.
>
> We shouldn't because the firmware is monitoring beacons and would cause a beacon miss
> indication, waking up the host.
No HTML please, our lists drop emails using HTML.
--
https://patchwork.kernel.org/project/linux-wireless/list/https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatc…
drm_master_release can be called on a drm_file without a master, which
results in a null ptr dereference of file_priv->master->magic_map. The
three cases are:
1. Error path in drm_open_helper
drm_open():
drm_open_helper():
drm_master_open():
drm_new_set_master(); <--- returns -ENOMEM,
drm_file.master not set
drm_file_free():
drm_master_release(); <--- NULL ptr dereference
(file_priv->master->magic_map)
2. Error path in mock_drm_getfile
mock_drm_getfile():
anon_inode_getfile(); <--- returns error, drm_file.master not set
drm_file_free():
drm_master_release(); <--- NULL ptr dereference
(file_priv->master->magic_map)
3. In drm_client_close, as drm_client_open doesn't set up a master
drm_file.master is set up in drm_open_helper through the call to
drm_master_open, so we mirror it with a call to drm_master_release in
drm_close_helper, and remove drm_master_release from drm_file_free to
avoid the null ptr dereference.
Fixes: 7eeaeb90a6a5 ("drm/file: Don't set master on in-kernel clients")
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx(a)gmail.com>
Cc: stable(a)vger.kernel.org
Reviewed-by: Daniel Vetter <daniel.vetter(a)ffwll.ch>
---
drivers/gpu/drm/drm_file.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index ed25168619fc..90b62f360da1 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -282,9 +282,6 @@ void drm_file_free(struct drm_file *file)
drm_legacy_ctxbitmap_flush(dev, file);
- if (drm_is_primary_client(file))
- drm_master_release(file);
-
if (dev->driver->postclose)
dev->driver->postclose(dev, file);
@@ -305,6 +302,9 @@ static void drm_close_helper(struct file *filp)
list_del(&file_priv->lhead);
mutex_unlock(&dev->filelist_mutex);
+ if (drm_is_primary_client(file_priv))
+ drm_master_release(file_priv);
+
drm_file_free(file_priv);
}
--
2.25.1