Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
From: "Irfan Ullah (울라 이르판)" <irfan@dke.khu.ac.kr>
To: "Valdis Klētnieks" <valdis.kletnieks@vt.edu>
Cc: Greg KH <greg@kroah.com>,
	Linux Kernel List <kernelnewbies@kernelnewbies.org>
Subject: Re: Netlink socket returns NULL in vmx.c kernel file
Date: Fri, 8 Nov 2019 13:54:47 +0900
Message-ID: <CA+mB8Oxw8OsFiSTzZZXXMKm_F-jTn3wGqNjDC_QgYWd0ydT4GA@mail.gmail.com> (raw)
In-Reply-To: <219753.1572950606@turing-police>

[-- Attachment #1.1: Type: text/plain, Size: 3071 bytes --]

Thank you very much. I am trying to rewrite the code. and fix these issues.

On Tue, Nov 5, 2019 at 7:43 PM Valdis Klētnieks <valdis.kletnieks@vt.edu>
wrote:

> On Tue, 05 Nov 2019 17:59:43 +0900, Irfan Ullah said:
>
> > Thank you for the response.
> > Attached are the files for kernel-user spaces communication.
>
> >               //when I remove this wait the code does not work
> >               msleep(3000);
>
> If your code doesn't work, but sticking in random delays makes
> it start working, you almost certainly have a race condition or
> synchronization issue that should be fixed using proper locking.
>
> > void hello_exit(void)
> > {
> >       //netlink_kernel_release(nl_sk);
>
> Congratulations. You just leaked a socket here, which is going to
> make it difficult to use that socket until you either reboot or find a
> way to close it properly before trying to create it again.
>
> > (code generates some warnings, but it is not severe and could be ignored
> for the time being).
>
> You should do the following:
> 1) Understand your code well enough so you understand *why* the compiler
> issued the warning.
> 2) Correct your code so the compiler doesn't complain. It almost certainly
> understands C better than you do.
>
> gcc 9.2.1 emits one warning on the kernel module code at default warning
> levels.  And it's one you *really* need to fix, because it indicates that
> you
> and the compiler are not on the same page about what types your variables
> are.
> Since it's going to go ahead and generate code based on what types *it*
> thinks
> your variables are, you will have nothing but pain and anguish debugging if
> you thought they were some other type....
>
> In fact, you may want to compile the kernel module with 'make W=1' to get
> more
> warnings.  If your system has sparse, you should use 'make W=1 C=1'.
>
> And all the warnings this generates are things that shouldn't be seen in
> clean
> kernel code.
>
> I didn't bother looking closely at your userspace.  I gave up
> when I saw this:
>
>      14 int sock_fd;
> (...)
>      68 void user_space_receiver()
>      69 {
> (...)
>      96                 user_space_receiver(sock_fd);
>
> There's 2 basic ways to pass a variable to a function. You're trying
> to use both of them here.  Pick one and use it properly.
>
> Oh - and there's no possible way to reach the close(sock_fd); on line
> 77, because the rest of the function infinite loops without a break.  At
> the
> very least, you should be checking the return code from recvmsg() and
> exiting the while(1) loop if there's an issue.
>
> Bottom line:  You need to get a *lot* more experience writing proper
> C code in userspace before you try to write kernel code.
>


-- 
*Best Regards,*


*Mr. Irfan Ullah*
PhD Candidate
Data and Knowledge Engineering(DKE) Lab
Department of Computer Science and Engineering
Kyung Hee University, South Korea.
 +82-010-591-51651 <+82%2010-3877-8867>
  sahibzada.iu@gmail.com
 sahibzada_irfanullah

[-- Attachment #1.2: Type: text/html, Size: 5264 bytes --]

<div dir="ltr">Thank you very much. I am trying to rewrite the code. and fix these issues. <br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 5, 2019 at 7:43 PM Valdis Klētnieks &lt;<a href="mailto:valdis.kletnieks@vt.edu">valdis.kletnieks@vt.edu</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, 05 Nov 2019 17:59:43 +0900, Irfan Ullah said:<br>
<br>
&gt; Thank you for the response.<br>
&gt; Attached are the files for kernel-user spaces communication.<br>
<br>
&gt;               //when I remove this wait the code does not work<br>
&gt;               msleep(3000);<br>
<br>
If your code doesn&#39;t work, but sticking in random delays makes<br>
it start working, you almost certainly have a race condition or<br>
synchronization issue that should be fixed using proper locking.<br>
<br>
&gt; void hello_exit(void)<br>
&gt; {<br>
&gt;       //netlink_kernel_release(nl_sk);<br>
<br>
Congratulations. You just leaked a socket here, which is going to<br>
make it difficult to use that socket until you either reboot or find a<br>
way to close it properly before trying to create it again.<br>
<br>
&gt; (code generates some warnings, but it is not severe and could be ignored for the time being).<br>
<br>
You should do the following:<br>
1) Understand your code well enough so you understand *why* the compiler<br>
issued the warning.<br>
2) Correct your code so the compiler doesn&#39;t complain. It almost certainly<br>
understands C better than you do.<br>
<br>
gcc 9.2.1 emits one warning on the kernel module code at default warning<br>
levels.  And it&#39;s one you *really* need to fix, because it indicates that you<br>
and the compiler are not on the same page about what types your variables are.<br>
Since it&#39;s going to go ahead and generate code based on what types *it* thinks<br>
your variables are, you will have nothing but pain and anguish debugging if<br>
you thought they were some other type....<br>
<br>
In fact, you may want to compile the kernel module with &#39;make W=1&#39; to get more<br>
warnings.  If your system has sparse, you should use &#39;make W=1 C=1&#39;.<br>
<br>
And all the warnings this generates are things that shouldn&#39;t be seen in clean<br>
kernel code.<br>
<br>
I didn&#39;t bother looking closely at your userspace.  I gave up<br>
when I saw this:<br>
<br>
     14 int sock_fd;<br>
(...)<br>
     68 void user_space_receiver()<br>
     69 {<br>
(...)<br>
     96                 user_space_receiver(sock_fd);<br>
<br>
There&#39;s 2 basic ways to pass a variable to a function. You&#39;re trying<br>
to use both of them here.  Pick one and use it properly.<br>
<br>
Oh - and there&#39;s no possible way to reach the close(sock_fd); on line<br>
77, because the rest of the function infinite loops without a break.  At the<br>
very least, you should be checking the return code from recvmsg() and<br>
exiting the while(1) loop if there&#39;s an issue.<br>
<br>
Bottom line:  You need to get a *lot* more experience writing proper<br>
C code in userspace before you try to write kernel code.<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><b style="color:rgb(39,78,19)">Best Regards,</b></div><div><b style="color:rgb(39,78,19)"><br></b></div><div><b style="color:rgb(39,78,19)"><br></b></div><div dir="ltr"><b style="color:rgb(39,78,19)">Mr. Irfan Ullah</b><br></div><div dir="ltr"><font color="#666666">PhD Candidate<br></font><div><font color="#666666">Data and Knowledge Engineering(DKE) Lab</font></div><div><font color="#666666">Department of Computer Science and Engineering</font></div><div><font color="#666666">Kyung Hee University, South Korea.</font></div><div><img src="http://teedayusa.com/media/wysiwyg/telephone_icon.gif" style="font-family: arial, sans-serif; font-size: 12.8px;"><span style="font-family:arial,sans-serif;font-size:12.8px"> </span><span style="font-family:arial,sans-serif;font-size:12.8px"><font color="#000000"><a href="tel:+82%2010-3877-8867" value="+821038778867" style="color:rgb(17,85,204)" target="_blank">+82-010-591-51651</a></font></span></div><div><div style="font-family:arial,sans-serif;font-size:12.8px"><img src="http://www.letsplaydodgeball.com/ndo/templates/dj-sport01/images/system/emailButton.png" style="font-size: 12.8px;"><span style="font-size:12.8px"><font color="#000000">  </font></span><span style="font-size:12.8px"><font color="#000000"><a href="mailto:sahibzada.iu@gmail.com" style="color:rgb(17,85,204)" target="_blank">sahibzada.iu@gmail.com</a></font></span></div><div><font face="arial, sans-serif"><span style="font-size:12.8px"><img src="https://docs.google.com/uc?export=download&amp;id=0B1-dY3m3XHQnOFBOM0NSODNoUUU&amp;revid=0B1-dY3m3XHQnUmp1c0ZWTGovR2pYbGZCenlOU29DY3ByQU5RPQ" style="font-size: 12.8px;"></span></font><span style="font-family:arial,sans-serif;font-size:12.8px"> </span><font face="arial, sans-serif"><span style="font-size:12.8px">sahibzada_irfanullah</span></font></div></div></div></div></div>

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

      reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-23  2:43 Irfan Ullah (울라 이르판)
2019-10-23  5:38 ` Valdis Klētnieks
2019-11-05  3:29   ` Irfan Ullah (울라 이르판)
2019-11-05  7:22     ` Greg KH
2019-11-05  8:59       ` Irfan Ullah (울라 이르판)
2019-11-05 10:43         ` Valdis Klētnieks
2019-11-08  4:54           ` Irfan Ullah (울라 이르판) [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+mB8Oxw8OsFiSTzZZXXMKm_F-jTn3wGqNjDC_QgYWd0ydT4GA@mail.gmail.com \
    --to=irfan@dke.khu.ac.kr \
    --cc=greg@kroah.com \
    --cc=kernelnewbies@kernelnewbies.org \
    --cc=valdis.kletnieks@vt.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org
	public-inbox-index kernelnewbies

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git