[v2] hwrng: Clean up RNG list when last hwrng is unregistered
diff mbox

Message ID 151336772989.60446.16322941485803241258.stgit@sosxen2.amd.com
State Accepted
Delegated to: Herbert Xu
Headers show

Commit Message

Gary R Hook Dec. 15, 2017, 7:55 p.m. UTC
Commit 142a27f0a731 added support for a "best" RNG, and in doing so
introduced a hang from rmmod/modprobe -r when the last RNG on the list
was unloaded.

When the hwrng list is depleted, return the global variables to their
original state and decrement all references to the object.

Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
Signed-off-by: Gary R Hook <gary.hook@amd.com>
---

Changes since v1: fix misspelled word in subject

 drivers/char/hw_random/core.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

PrasannaKumar Muralidharan Dec. 17, 2017, 9:23 a.m. UTC | #1
Hi Gary,

Some minor comments below.

On 16 December 2017 at 01:25, Gary R Hook <gary.hook@amd.com> wrote:
>
> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
> introduced a hang from rmmod/modprobe -r when the last RNG on the list
> was unloaded.

Nice catch. Thanks for fixing this.

> When the hwrng list is depleted, return the global variables to their
> original state and decrement all references to the object.
>
> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")

Please cc the commit author (in this case its me) so that this patch
gets noticed easily.

> Signed-off-by: Gary R Hook <gary.hook@amd.com>
> ---
>
> Changes since v1: fix misspelled word in subject
>
>  drivers/char/hw_random/core.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 657b8770b6b9..91bb98c42a1c 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
>                 ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
>                 if (!ret)
>                         cur_rng_set_by_user = 0;
> +       } else {
> +               drop_current_rng();

When the hwrng list is empty just set current_rng = NULL instead of
calling drop_current_rng().

> +               cur_rng_set_by_user = 0;
> +               ret = 0;
>         }
>
>         return ret;
>

Regards,
PrasannaKumar
PrasannaKumar Muralidharan Dec. 17, 2017, 9:49 a.m. UTC | #2
On 17 December 2017 at 14:53, PrasannaKumar Muralidharan
<prasannatsmkumar@gmail.com> wrote:
> Hi Gary,
>
> Some minor comments below.
>
> On 16 December 2017 at 01:25, Gary R Hook <gary.hook@amd.com> wrote:
>>
>> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
>> introduced a hang from rmmod/modprobe -r when the last RNG on the list
>> was unloaded.
>
> Nice catch. Thanks for fixing this.
>
>> When the hwrng list is depleted, return the global variables to their
>> original state and decrement all references to the object.
>>
>> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
>
> Please cc the commit author (in this case its me) so that this patch
> gets noticed easily.
>
>> Signed-off-by: Gary R Hook <gary.hook@amd.com>
>> ---
>>
>> Changes since v1: fix misspelled word in subject
>>
>>  drivers/char/hw_random/core.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>> index 657b8770b6b9..91bb98c42a1c 100644
>> --- a/drivers/char/hw_random/core.c
>> +++ b/drivers/char/hw_random/core.c
>> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
>>                 ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
>>                 if (!ret)
>>                         cur_rng_set_by_user = 0;
>> +       } else {
>> +               drop_current_rng();
>
> When the hwrng list is empty just set current_rng = NULL instead of
> calling drop_current_rng().
>
>> +               cur_rng_set_by_user = 0;
>> +               ret = 0;
>>         }
>>
>>         return ret;
>>
>
> Regards,
> PrasannaKumar

I am fine with the code as is.

Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>

Regards,
PrasannaKumar
Gary R Hook Dec. 19, 2017, 5:52 p.m. UTC | #3
On 12/17/2017 03:49 AM, PrasannaKumar Muralidharan wrote:
> On 17 December 2017 at 14:53, PrasannaKumar Muralidharan
> <prasannatsmkumar@gmail.com> wrote:
>> Hi Gary,
>>
>> Some minor comments below.
>>
>> On 16 December 2017 at 01:25, Gary R Hook <gary.hook@amd.com> wrote:
>>>
>>> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
>>> introduced a hang from rmmod/modprobe -r when the last RNG on the list
>>> was unloaded.
>>
>> Nice catch. Thanks for fixing this.
>>
>>> When the hwrng list is depleted, return the global variables to their
>>> original state and decrement all references to the object.
>>>
>>> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
>>
>> Please cc the commit author (in this case its me) so that this patch
>> gets noticed easily.

D'oh! I did not do so on this version. My apologies.

>>
>>> Signed-off-by: Gary R Hook <gary.hook@amd.com>
>>> ---
>>>
>>> Changes since v1: fix misspelled word in subject
>>>
>>>   drivers/char/hw_random/core.c |    4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>>> index 657b8770b6b9..91bb98c42a1c 100644
>>> --- a/drivers/char/hw_random/core.c
>>> +++ b/drivers/char/hw_random/core.c
>>> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
>>>                  ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
>>>                  if (!ret)
>>>                          cur_rng_set_by_user = 0;
>>> +       } else {
>>> +               drop_current_rng();
>>
>> When the hwrng list is empty just set current_rng = NULL instead of
>> calling drop_current_rng().
>>
>>> +               cur_rng_set_by_user = 0;
>>> +               ret = 0;
>>>          }
>>>
>>>          return ret;
>>>
>>
>> Regards,
>> PrasannaKumar
> 
> I am fine with the code as is.
> 
> Reviewed-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> 
> Regards,
> PrasannaKumar
>
Herbert Xu Dec. 22, 2017, 9:12 a.m. UTC | #4
On Fri, Dec 15, 2017 at 01:55:59PM -0600, Gary R Hook wrote:
> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
> introduced a hang from rmmod/modprobe -r when the last RNG on the list
> was unloaded.
> 
> When the hwrng list is depleted, return the global variables to their
> original state and decrement all references to the object.
> 
> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
> Signed-off-by: Gary R Hook <gary.hook@amd.com>

Patch applied.  Thanks.
Gary R Hook Jan. 5, 2018, 5:28 p.m. UTC | #5
On 12/15/2017 01:55 PM, Gary R Hook wrote:
> Commit 142a27f0a731 added support for a "best" RNG, and in doing so
> introduced a hang from rmmod/modprobe -r when the last RNG on the list
> was unloaded.
> 
> When the hwrng list is depleted, return the global variables to their
> original state and decrement all references to the object.

*ping*

It may not have been obvious from the title but this fixes a bug which 
will impact the use of any HW RNG that is the only RNG registered. The 
breakage of rmmod/modprobe -r that this fix obviates suggests that it 
needs to make the 4.15 kernel, please. Just want to ensure this gets the 
proper attention. Thank you.


> 
> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current")
> Signed-off-by: Gary R Hook <gary.hook@amd.com>
> ---
> 
> Changes since v1: fix misspelled word in subject
> 
>   drivers/char/hw_random/core.c |    4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 657b8770b6b9..91bb98c42a1c 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -306,6 +306,10 @@ static int enable_best_rng(void)
>   		ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
>   		if (!ret)
>   			cur_rng_set_by_user = 0;
> +	} else {
> +		drop_current_rng();
> +		cur_rng_set_by_user = 0;
> +		ret = 0;
>   	}
>   
>   	return ret;
>
[Desktop Entry]
Encoding=UTF-8
Name=Thunderbird Mail
Name[ast]=Veceru de corréu Thunderbird
Name[ca]=Client de correu Thunderbird
Name[cs]=Poštovní klient Thunderbird
Name[da]=Mozilla Thunderbird - e-post/nyhedsgruppe
Name[de]=Thunderbird-E-Mail und -Nachrichten
Name[el]=Ηλεκτρονική αλληλογραφία Thunderbird
Name[es]=Cliente de correo Thunderbird
Name[fi]=Thunderbird-sähköposti
Name[fr]=Messagerie Thunderbird
Name[gl]=Cliente de correo Thunderbird
Name[he]=Mozilla Thunderbird דוא״ל/חדשות
Name[hr]=Mozilla Thunderbird e-pošta/novosti
Name[hu]=Thunderbird levelezőkliens
Name[it]=Email Mozilla Thunderbird
Name[ja]=Thunderbird電子メールクライアント
Name[ko]=Mozilla Thunderbird
Name[nl]=Mozilla Thunderbird e-mail/nieuws
Name[pl]=Klient poczty Thunderbird
Name[pt_BR]=Cliente de E-mail Thunderbird
Name[ru]=Почтовый клиент Thunderbird
Name[sk]=Thunderbird - poštový klient
Name[sv]=E-postklienten Thunderbird
Name[ug]=Mozilla Thunderbird ئېلخەت/خەۋەر
Name[uk]=Поштова програма Thunderbird
Name[vi]=Trình điện thư Mozilla Thunderbird
Name[zh_CN]=Thunderbird 邮件/新闻
Name[zh_TW]=Thunderbird 郵件
Comment=Send and receive mail with Thunderbird
Comment[ast]=Lleer y escribir corréu electrónicu
Comment[ca]=Llegiu i escriviu correu
Comment[cs]=Čtení a psaní pošty
Comment[da]=Skriv/læs e-post/nyhedsgruppe med Mozilla Thunderbird
Comment[de]=E-Mails und Nachrichten mit Thunderbird lesen und schreiben
Comment[el]=Διαβάστε και γράψτε γράμματα με το Mozilla Thunderbird
Comment[es]=Lea y escriba correos y noticias con Thunderbird
Comment[fi]=Lue ja kirjoita sähköposteja
Comment[fr]=Lire et écrire des courriels
Comment[gl]=Lea e escriba correo electrónico
Comment[he]=קריאה/כתיבה של דוא״ל/חדשות באמצעות Mozilla Thunderbird
Comment[hr]=Čitajte/šaljite e-poštu s Thunderbird
Comment[hu]=Levelek írása és olvasása a Thunderbirddel
Comment[it]=Per leggere e scrivere email
Comment[ja]=メールの読み書き
Comment[ko]=Mozilla Thunderbird 메일/뉴스 읽기 및 쓰기 클라이언트
Comment[nl]=E-mail/nieuws lezen en schrijven met Mozilla Thunderbird
Comment[pl]=Czytanie i wysyłanie e-maili
Comment[pt_BR]=Leia e escreva suas mensagens
Comment[ru]=Читайте и пишите письма
Comment[sk]=Čítajte a píšte poštu pomocou programu Thunderbird
Comment[sv]=Läs och skriv e-post
Comment[ug]=ئېلخەت ۋە خەۋەرلەرنى Mozilla Thunderbird دا كۆرۈش ۋە يېزىش
Comment[uk]=Читання та написання листів
Comment[vi]=Đọc và soạn thư điện tử
Comment[zh_CN]=阅读邮件或新闻
Comment[zh_TW]=以 Mozilla Thunderbird 讀寫郵件或新聞
GenericName=Mail Client
GenericName[ast]=Client de correu
GenericName[ca]=Client de correu
GenericName[cs]=Poštovní klient
GenericName[da]=E-postklient
GenericName[de]=E-Mail-Anwendung
GenericName[el]=Λογισμικό αλληλογραφίας
GenericName[es]=Cliente de correo
GenericName[fi]=Sähköpostiohjelma
GenericName[fr]=Client de messagerie
GenericName[gl]=Cliente de correo electrónico
GenericName[he]=לקוח דוא״ל
GenericName[hr]=Klijent e-pošte
GenericName[hu]=Levelezőkliens
GenericName[it]=Client email
GenericName[ja]=電子メールクライアント
GenericName[ko]=메일 클라이언트
GenericName[nl]=E-mailprogramma
GenericName[pt_BR]=Cliente de E-mail
GenericName[ru]=Почтовый клиент
GenericName[sk]=Poštový klient
GenericName[ug]=ئېلخەت دېتالى
GenericName[uk]=Поштова програма
GenericName[vi]=Phần mềm khách quản lý thư điện tử
GenericName[zh_CN]=邮件新闻客户端
GenericName[zh_TW]=郵件用戶端
Keywords=Email;E-mail;Newsgroup;Feed;RSS
Keywords[ast]=Corréu;Corréu-e;Noticies;Discusiones;Mensaxes;Canales;RSS
Keywords[ca]=Correu;Email;E-mail;Mailing;Llistes;Notícies;RSS
Keywords[cs]=Email;E-mail;Pošta;Elektronická pošta;Diskusní skupiny;Čtečka;RSS
Keywords[da]=Email;E-mail;Epost;E-post;E-brev;Newsgroup;Nyhedsgruppe;Nyhedskilde;Usenet;Feed;RSS
Keywords[de]=Mail;E-Mail;Newsgroup;Nachrichten;Feed;RSS;Post;News;Usenet;online;lesen;schreiben
Keywords[el]=Email;E-mail;Newsgroup;Feed;RSS;ημαιλ;Αλληλογραφία;Ροή;ΡΣΣ;Συζητήσεις;Γράμματα
Keywords[es]=Email;Correo electrónico;Noticias;Discusiones;Mensajes;Canales;RSS
Keywords[fi]=Mail;E-Mail;Email;Newsgroup;Feed;RSS;posti;sähköposti;maili;meili;Usenet;uutisryhmät;syöte
Keywords[fr]=Mails;Mels;E-Mails;Emails;Courriels;Courriers;Newsgroup;Feed;RSS;Poster;Thunderbird;Lire;Écrire
Keywords[he]=דוא"ל;דוא״ל;מייל;אי-מייל;אימייל;הודעות;מוזילה;תאנדרבירד;ת׳אנדרבירד;ת'אנדרבירד;ת׳אנדרברד;ת'אנדרברד;דואל;
Keywords[hr]=email;e-mail;e-pošta;pošta;RSS
Keywords[hu]=Email;E-mail;Levél;Levelezés;Hírcsoport;Feed;Hírforrás;RSS
Keywords[it]=Email;Mail;Posta;Newsgroup;Feed;RSS
Keywords[is]=tölvupóstur;rafpóstur;fréttir;fréttahópar;samtöl;skilaboð;fréttastraumar;RSS
Keywords[ja]=Eメール;イーメール;mail;e-mail;email;メール;電子メール;ニュースグループ;ネットニュース;RSS;フィードリーダー;書く;読む;Mozilla
Keywords[nl]=Email;E-mail;Newsgroup;Feed;RSS;Nieuwsgroep;Post
Keywords[ru]=Email;E-mail;Newsgroup;Feed;RSS;почта;письма;новости;фиды
Keywords[sk]=Email;E-mail;Elektronická pošta;Diskusné skupiny;Čítačka kanálov;RSS
Keywords[uk]=Email;E-mail;Newsgroup;Feed;RSS;пошта;новини;подачі;стрічка
Keywords[vi]=Mail;E-Mail;Email;Newsgroup;Feed;RSS;Thư điện tử;Gửi thư
Keywords[zh_CN]=Mail;E-Mail;Email;Newsgroup;Feed;RSS;电子;邮件;新闻;Thunderbird;tb;雷鸟;电邮;邮箱;阅读器;
Exec=thunderbird %u
Terminal=false
X-MultipleArgs=false
Type=Application
Icon=thunderbird
Categories=Application;Network;Email;
MimeType=x-scheme-handler/mailto;application/x-xpinstall;
StartupNotify=true
Actions=Compose;Contacts

[Desktop Action Compose]
Name=Compose New Message
Name[af]=Skryf 'n nuwe boodskap
Name[ar]=حرر رسالة جديدة
Name[ast]=Redactar un mensaxe nuevu
Name[be]=Напісаць ліст
Name[bg]=Ново писмо
Name[bn]=নতুন বার্তা লিখুন
Name[br]=Sevel ur gemennadenn nevez
Name[bs]=Napiši novu poruku
Name[ca]=Redacta un missatge nou
Name[cs]=Napsat novou zprávu
Name[cy]=Cyfansoddi Neges Newydd
Name[da]=Skriv en ny besked
Name[de]=Neue Nachricht erstellen
Name[el]=Σύνταξη νέου μηνύματος
Name[eo]=Redakti novan mesaĝon
Name[es]=Redactar un mensaje nuevo
Name[et]=Koosta uus sõnum
Name[eu]=Mezu Berria Idatzi
Name[fi]=Luo uusi viesti
Name[fr]=Rédiger un nouveau courriel
Name[gd]=Co-chuir Teachdaireachd Ùr
Name[gl]=Redactar unha nova mensaxe
Name[he]=חיבור הודעה חדשה
Name[hi]=नया संदेश लिखें
Name[hr]=Nova poruka
Name[hu]=Új üzenet írása
Name[hy]=Նամակ գրել
Name[id]=Buat Pesan Baru
Name[is]=Skrifa nýtt bréf
Name[it]=Componi nuovo messaggio
Name[ja]=新しいメッセージの作成
Name[kk]=Жаңа хат жазу
Name[kn]=ಹೊಸ ಸಂದೇಶವನ್ನು ಸಂಯೋಜಿಸು
Name[ko]=새 메시지 쓰기
Name[ku]=Peyameke nû biafirînê
Name[lt]=Rašyti naują laišką
Name[lv]=Sacerēt jaunu vēstuli
Name[mk]=Состави нова порака
Name[ml]=പുതിയ സന്ദേശം രചിക്കുക
Name[mr]=नवीन संदेश लिहा.
Name[nb]=Skriv en ny melding
Name[nl]=Nieuw bericht opstellen
Name[nn]=Skriv ei ny melding
Name[pl]=Utwórz nową wiadomość
Name[pt]=Criar nova mensagem
Name[pt_BR]=Compor nova mensagem
Name[ro]=Compune un mesaj nou
Name[ru]=Написать письмо
Name[si]=නව පණිවිඩයක් අරඹන්න
Name[sk]=Napísať novú správu
Name[sl]=Sestavi novo sporočilo
Name[sq]=Krijo Mesazh të Ri
Name[sr]=Састави нову поруку
Name[sv]=Skriv nytt meddelande
Name[ta]=புதிய செய்தியை உருவாக்கு
Name[te]=కొత్త సందేశాన్ని రచించు
Name[th]=สร้างข้อความใหม่
Name[tr]=Yeni İleti Oluştur
Name[uk]=Написати нове повідомлення
Name[zh_CN]=撰写新消息
Name[zh_TW]=撰寫新郵件
Exec=thunderbird -compose
OnlyShowIn=Messaging Menu;Unity;

[Desktop Action Contacts]
Name=Contacts
Name[af]=Kontakte
Name[ar]=المتراسلين
Name[ast]=Contautos
Name[be]=Кантакты
Name[bg]=Контакти
Name[bn]=পরিচিতি সমূহ
Name[br]=Darempredoù
Name[bs]=Kontakti
Name[ca]=Contactes
Name[cs]=Kontakty
Name[cy]=Cysylltiadau
Name[da]=Kontakter
Name[de]=Kontakte
Name[el]=Επαφές
Name[eo]=Kontaktoj
Name[es]=Contactos
Name[et]=Kontaktid
Name[eu]=Kontaktuak
Name[fi]=Yhteystiedot
Name[fr]=Carnet d'adresses
Name[gd]=Buntanasan
Name[gl]=Contactos
Name[he]=אנשי קשר
Name[hi]=संपर्क
Name[hr]=Kontakti
Name[hu]=Névjegyek
Name[hy]=Կապորդներ
Name[id]=Kontak
Name[is]=Tengiliðir
Name[it]=Contatti
Name[ja]=連絡先
Name[kk]=Байланыс жазулары
Name[kn]=ಸಂಪರ್ಕಗಳು
Name[ko]=연락처
Name[ku]=Tekilî
Name[lt]=Kontaktai
Name[lv]=Kontakti
Name[mk]=Контакти
Name[ml]=വിലാസങ്ങള്‍
Name[mr]=संपर्क
Name[nb]=Kontakter
Name[nl]=Contacten
Name[nn]=Kontaktar
Name[pl]=Kontakty
Name[pt]=Contactos
Name[pt_BR]=Contatos
Name[ro]=Contacte
Name[ru]=Контакты
Name[si]=සබඳතා
Name[sk]=Kontakty
Name[sl]=Stiki
Name[sq]=Kontaktet
Name[sr]=Контакти
Name[sv]=Kontakter
Name[ta]=தொடர்புகள்
Name[te]=పరిచయాలు
Name[th]=รายชื่อติดต่อ
Name[tr]=Kişiler
Name[uk]=Контакти
Name[zh_CN]=联系人
Name[zh_TW]=連絡人
Exec=thunderbird -addressbook
OnlyShowIn=Messaging Menu;Unity;
Herbert Xu Jan. 8, 2018, 5:05 a.m. UTC | #6
On Fri, Jan 05, 2018 at 11:28:23AM -0600, Gary R Hook wrote:
> 
> It may not have been obvious from the title but this fixes a bug
> which will impact the use of any HW RNG that is the only RNG
> registered. The breakage of rmmod/modprobe -r that this fix obviates
> suggests that it needs to make the 4.15 kernel, please. Just want to
> ensure this gets the proper attention. Thank you.

I don't think breaking rmmod is sufficiently serious to warrant
immediate inclusion at this point.  We can always send it via
stable after the release.

Thanks,
Gary R Hook Jan. 8, 2018, 2:38 p.m. UTC | #7
On 01/07/2018 11:05 PM, Herbert Xu wrote:
> On Fri, Jan 05, 2018 at 11:28:23AM -0600, Gary R Hook wrote:
>>
>> It may not have been obvious from the title but this fixes a bug
>> which will impact the use of any HW RNG that is the only RNG
>> registered. The breakage of rmmod/modprobe -r that this fix obviates
>> suggests that it needs to make the 4.15 kernel, please. Just want to
>> ensure this gets the proper attention. Thank you.
> 
> I don't think breaking rmmod is sufficiently serious to warrant
> immediate inclusion at this point.  We can always send it via
> stable after the release.
> 
> Thanks,
> 

Oh. I just figured that problems introduced in a particular kernel level 
should be resolved (if possible) in that same level.

Your call, of course; thank you for the clarification.

Patch
diff mbox

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index 657b8770b6b9..91bb98c42a1c 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -306,6 +306,10 @@  static int enable_best_rng(void)
 		ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng));
 		if (!ret)
 			cur_rng_set_by_user = 0;
+	} else {
+		drop_current_rng();
+		cur_rng_set_by_user = 0;
+		ret = 0;
 	}
 
 	return ret;